diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index cf197c732..6451b0d9b 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -3818,7 +3818,22 @@ export class LocalBackend { } else if (isQualified) { // Parenthesised because the kind filter below is appended with AND, which // binds tighter than OR. - whereClause = `WHERE (n.id = $symName OR n.name = $symName)`; + // #3074: a repo-relative file path (e.g. "supabase/functions/_shared/crypto.ts") + // is the most natural way to name a File and is exactly what `target.filePath` + // reports, but the old clause only matched `n.id` (= "File:") or basename + // `n.name`, so the same path the graph stores never resolved. Also match the + // repo-relative `n.filePath` exactly and via an anchored suffix (segment-boundary + // "ENDS WITH $suffix" where suffix is "/"+path) so "a.ts" does not spuriously + // match "mylib/a.ts" — same anchoring used in detect_changes (#2915). + const suffix = pathSuffixOf(name); + // File-path terms must be scoped to File nodes — n.filePath is shared by + // every symbol in the file, so an unlabeled predicate would turn + // "src/actions.ts" into every symbol in that file (bot review #3084 P1). + // LadybugDB does not allow label tests in WHERE (n:File), so scope via + // id prefix — File nodes are `File:`. + whereClause = `WHERE (n.id = $symName OR n.name = $symName OR (n.id STARTS WITH $filePrefix AND (n.filePath = $symName OR n.filePath ENDS WITH $suffix)))`; + queryParams.suffix = suffix; + queryParams.filePrefix = 'File:'; } else { whereClause = `WHERE n.name = $symName`; } @@ -3909,7 +3924,7 @@ export class LocalBackend { if (rows.length === 0) return { kind: 'not_found' }; // Normalise row shape across object / tuple returns from LadybugDB. - const normalized = rows.map((r: any) => ({ + let normalized = rows.map((r: any) => ({ id: (r.id ?? r[0]) as string, name: (r.name ?? r[1]) as string, type: (r.type ?? r[2] ?? '') as string, @@ -3919,6 +3934,16 @@ export class LocalBackend { ...(include_content ? { content: (r.content ?? r[6]) as string | undefined } : {}), })); + // An exact File path wins over anchored suffix candidates. Without this, + // `lib/a.ts` and `src/lib/a.ts` both score as File candidates and turn an + // otherwise unambiguous exact target into `ambiguous` (#3084 review P2). + if (isQualified) { + const exactFiles = normalized.filter( + (candidate) => candidate.id.startsWith('File:') && candidate.filePath === name, + ); + if (exactFiles.length > 0) normalized = exactFiles; + } + // The COUNT can never legitimately be below the page it accompanies, so a // value under `normalized.length` means the count leg failed or returned an // unreadable shape. Keep the window size as the floor — reporting zero would @@ -6127,6 +6152,7 @@ export class LocalBackend { direction: params.direction, suggestion, recoverySuggestion, + undetermined: true, }); return pdgErr; } @@ -6134,7 +6160,7 @@ export class LocalBackend { error: message, target: { name: params.target }, direction: params.direction, - impactedCount: 0, + impactedCount: null, risk: 'UNKNOWN', suggestion, ...(recoverySuggestion ? { recoverySuggestion } : {}), @@ -6229,6 +6255,7 @@ export class LocalBackend { `(single-repo PDG impact). Remove them or use mode:'callgraph' for cross-repo fan-out.`, target: crossDepthTarget, direction, + undetermined: true, }); return pdgErr; } @@ -6295,12 +6322,17 @@ export class LocalBackend { error: `Target '${missing}' not found`, target: notFoundTarget, direction, + undetermined: true, }) : { error: `Target '${missing}' not found`, target: { name: target }, direction, - impactedCount: 0, + // #3074 follow-up: do not ship a normal-shaped 0/UNKNOWN blast radius + // alongside the error — it reads as a real "nothing depends on this" + // answer. Null marks UNDETERMINED (same as the ambiguous path) so a + // consumer testing `impactedCount === 0` cannot misread a miss as safe. + impactedCount: null, risk: 'UNKNOWN', }; } diff --git a/gitnexus/src/mcp/local/pdg-impact.ts b/gitnexus/src/mcp/local/pdg-impact.ts index d2de59629..4cd311be1 100644 --- a/gitnexus/src/mcp/local/pdg-impact.ts +++ b/gitnexus/src/mcp/local/pdg-impact.ts @@ -118,8 +118,10 @@ export function splitCalleeIds(raw: unknown): string[] { * Bump on any breaking change to the PDG result fields. * v2: `startLine` in the result is now 1-based display (#2380), matching the * context/query/impact tools (was 0-based). + * v3: error envelopes use `impactedCount: null` when a target cannot be resolved + * (#3074), so consumers cannot read a miss as a measured zero. */ -export const PDG_RESULT_VERSION = 2 as const; +export const PDG_RESULT_VERSION = 3 as const; /** A reachable dependence block resolved to its source statement. */ export interface PdgStatement { @@ -742,7 +744,7 @@ export interface PdgInterproceduralImpact { export interface PdgImpactBaseResult extends PdgImpactParityFields { mode: 'pdg'; /** Contract version of the mode:'pdg' impact result shape; bump on any breaking change to the PDG result fields. */ - pdgResultVersion: 2; + pdgResultVersion: 3; target: PdgImpactTarget; direction: 'upstream' | 'downstream'; impactedCount: number; @@ -815,11 +817,11 @@ export interface PdgImpactDegradedResult extends PdgImpactBaseResult { export interface PdgImpactErrorResult { mode?: 'pdg'; /** Contract version of the mode:'pdg' impact result shape; bump on any breaking change to the PDG result fields. */ - pdgResultVersion: 2; + pdgResultVersion: 3; error: string; target: PdgImpactTarget; direction: 'upstream' | 'downstream'; - impactedCount: 0; + impactedCount: number | null; risk: 'UNKNOWN'; suggestion?: string; recoverySuggestion?: string; @@ -838,6 +840,8 @@ export function makePdgImpactErrorResult(input: { mode?: 'pdg'; suggestion?: string; recoverySuggestion?: string; + /** True when analysis did not obtain a measured impact count. */ + undetermined?: boolean; }): PdgImpactErrorResult { return { ...(input.mode ? { mode: input.mode } : {}), @@ -845,7 +849,10 @@ export function makePdgImpactErrorResult(input: { error: input.error, target: input.target, direction: input.direction, - impactedCount: 0, + // #3074 follow-up + P2 review: an unmeasured PDG result must not ship a + // confident-looking 0 blast radius. null marks UNDETERMINED, so a consumer + // testing === 0 cannot misread a miss or failed query as safe. + impactedCount: input.undetermined ? null : 0, risk: 'UNKNOWN', ...(input.suggestion ? { suggestion: input.suggestion } : {}), ...(input.recoverySuggestion ? { recoverySuggestion: input.recoverySuggestion } : {}), diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index be793cbeb..53700a7a2 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -469,7 +469,7 @@ 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 in affectedStatements (line + text). Inter-procedural symbols are still reported through interproceduralByDepth/pdgInterprocedural and the compatibility byDepth bucket. Without "line", pdg returns whole-symbol inter-procedural reach plus local whole-symbol PDG diagnostics. -PDG OUTPUT CONTRACT: every mode:'pdg' result (success, empty, degraded, or error) carries pdgResultVersion:2 — a stable discriminator for external consumers that bumps on any breaking change to the PDG result shape (distinct from the DB schema version). Successful PDG results include mode:'pdg', a full target envelope (id/name/type/filePath), affectedStatements, affectedStatementCount, interproceduralByDepth/pdgInterprocedural for cross-function reach, compatibility byDepth/byDepthCounts, risk:'UNKNOWN', and a note describing the unified contract. Degraded PDG results (no-layer, sub-layer-missing, unknown) keep mode:'pdg', pdgResultVersion:2, 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. Return-value-ascent coverage is published structurally at pdgEvidence.ascent — present iff the inter-procedural descent ran, including on an empty slice — with referencesScanned (DISTINCT callees scanned for a CALL_SUMMARY: a distinct-id tally, not a call-site count — two call sites to the same callee count once), returnFlowFound (whether the ascent fired anywhere in the slice), undecodableSummaryCount, examinedComplete (whether that scan covered every callee the index recorded a resolved id for on the visited blocks), incompleteReasons ('traversal-truncated' | 'callee-list-capped' | 'callee-ids-unrecorded'), and callSummaryLayerPresent. Read callSummaryLayerPresent FIRST: false ⇒ a pre-CALL_SUMMARY index, so {referencesScanned:N>0, returnFlowFound:false} is self-consistent and says nothing about the callees — the scan ran, but no layer existed in which a return-flow could be recorded (remedy: re-run gitnexus analyze --pdg). Branch on those fields; the note narrates the same facts in prose for humans and is not a stable contract. +PDG OUTPUT CONTRACT: every mode:'pdg' result (success, empty, degraded, or error) carries pdgResultVersion:3 — a stable discriminator for external consumers that bumps on any breaking change to the PDG result shape (distinct from the DB schema version). Successful PDG results include mode:'pdg', a full target envelope (id/name/type/filePath), affectedStatements, affectedStatementCount, interproceduralByDepth/pdgInterprocedural for cross-function reach, compatibility byDepth/byDepthCounts, risk:'UNKNOWN', and a note describing the unified contract. Degraded PDG results (no-layer, sub-layer-missing, unknown) keep mode:'pdg', pdgResultVersion:3, 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. Return-value-ascent coverage is published structurally at pdgEvidence.ascent — present iff the inter-procedural descent ran, including on an empty slice — with referencesScanned (DISTINCT callees scanned for a CALL_SUMMARY: a distinct-id tally, not a call-site count — two call sites to the same callee count once), returnFlowFound (whether the ascent fired anywhere in the slice), undecodableSummaryCount, examinedComplete (whether that scan covered every callee the index recorded a resolved id for on the visited blocks), incompleteReasons ('traversal-truncated' | 'callee-list-capped' | 'callee-ids-unrecorded'), and callSummaryLayerPresent. Read callSummaryLayerPresent FIRST: false ⇒ a pre-CALL_SUMMARY index, so {referencesScanned:N>0, returnFlowFound:false} is self-consistent and says nothing about the callees — the scan ran, but no layer existed in which a return-flow could be recorded (remedy: re-run gitnexus analyze --pdg). Branch on those fields; the note narrates the same facts in prose for humans and is not a stable contract. 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. diff --git a/gitnexus/test/integration/impact-pdg-callsummary-degradation.test.ts b/gitnexus/test/integration/impact-pdg-callsummary-degradation.test.ts index ee9b31c92..eb43fc926 100644 --- a/gitnexus/test/integration/impact-pdg-callsummary-degradation.test.ts +++ b/gitnexus/test/integration/impact-pdg-callsummary-degradation.test.ts @@ -17,7 +17,7 @@ * "complete" result. * * This golden asserts the EXACT degraded envelope (not just non-crash): - * - the result is still mode:'pdg' with pdgResultVersion:2 (the contract + * - the result is still mode:'pdg' with pdgResultVersion:3 (the contract * discriminator); * - the intra slice is PRESENT (CALL_SUMMARY is NOT a required sub-layer — the * index is `ready`, pdgLayer is undefined, risk is UNKNOWN, epistemic is the @@ -75,14 +75,14 @@ withTestLbugDB( }); describe('CALL_SUMMARY-absent (v3 / pre-FU-C index): the ascent is silent but the user is TOLD', () => { - it('returns the EXACT degraded envelope — mode:pdg, pdgResultVersion:2, intra slice present, risk UNKNOWN', async () => { + it('returns the EXACT degraded envelope — mode:pdg, pdgResultVersion:3, intra slice present, risk UNKNOWN', async () => { const result = await slice(); // Golden envelope: the index is `ready` (CALL_SUMMARY is NOT a required // sub-layer), so this is a real traversal result — NOT a pdgLayer // degradation early-return. The intra slice ran and risk stays UNKNOWN. expect(result).toMatchObject({ mode: 'pdg', - pdgResultVersion: 2, + pdgResultVersion: 3, risk: 'UNKNOWN', epistemic: 'pdg-intra-procedural', target: { id: 'func:fnA', name: 'fnA' }, diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index e3f1892ce..f5693f090 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -1344,12 +1344,67 @@ describe('LocalBackend.callTool', () => { await backend.callTool('context', { name: 'src/a.ts:collide', kind: 'Function' }); const parenthesised = - /WHERE \(n\.id = \$symName OR n\.name = \$symName\) AND n\.id STARTS WITH \$kindPrefix/; + /WHERE \(n\.id = \$symName OR n\.name = \$symName OR \(n\.id STARTS WITH \$filePrefix AND \(n\.filePath = \$symName OR n\.filePath ENDS WITH \$suffix\)\)\) AND n\.id STARTS WITH \$kindPrefix/; const calls = resolverCalls(); expect(calls).toHaveLength(2); expect(calls.filter((c) => parenthesised.test(c.query))).toHaveLength(2); }); + it('exact File path wins over suffixed matches during qualified resolution (#3084 review P2)', async () => { + (executeParameterized as any).mockImplementation(async (_repo: string, query: string) => { + if (query.startsWith('MATCH (n)')) { + return [ + { + id: 'File:src/lib/a.ts', + name: 'a.ts', + filePath: 'src/lib/a.ts', + kind: 'File', + total_hits: 1, + }, + { + id: 'File:lib/a.ts', + name: 'a.ts', + filePath: 'lib/a.ts', + kind: 'File', + total_hits: 1, + }, + ]; + } + return [{ total: 2 }]; + }); + + const result = await backend.callTool('context', { name: 'lib/a.ts' }); + expect(result).toMatchObject({ + status: 'found', + symbol: { + filePath: 'lib/a.ts', + uid: 'File:lib/a.ts', + }, + }); + }); + + it('not_found impact queries return impactedCount null and risk UNKNOWN across modes (#3074 / #3084 review)', async () => { + (executeParameterized as any).mockImplementation(async () => []); + + const cgResult = await backend.callTool('impact', { target: 'nonexistent_target_xyz' }); + expect(cgResult).toMatchObject({ + error: "Target 'nonexistent_target_xyz' not found", + impactedCount: null, + risk: 'UNKNOWN', + }); + + const pdgResult = await backend.callTool('impact', { + target: 'nonexistent_target_xyz', + mode: 'pdg', + }); + expect(pdgResult).toMatchObject({ + error: "Target 'nonexistent_target_xyz' not found", + impactedCount: null, + risk: 'UNKNOWN', + pdgResultVersion: 3, + }); + }); + it('retries UNFILTERED when the kind hint matches no label prefix (#2787 review F5)', async () => { // `kind` is a free-form string on the tool schema. A miscased or // repo-absent kind must not turn a real name into `not_found` — the @@ -3241,7 +3296,7 @@ describe('LocalBackend impact mode (KTD1/KTD5/KTD12)', () => { expect(result.mode).toBe('pdg'); expect(result.target).toEqual({ name: 'missingSymbol' }); expect(result.direction).toBe('upstream'); - expect(result.impactedCount).toBe(0); + expect(result.impactedCount).toBeNull(); expect(result.risk).toBe('UNKNOWN'); }); @@ -3257,7 +3312,7 @@ describe('LocalBackend impact mode (KTD1/KTD5/KTD12)', () => { expect(result.mode).toBe('pdg'); expect(result.target).toEqual({ name: 'main' }); expect(result.direction).toBe('downstream'); - expect(result.impactedCount).toBe(0); + expect(result.impactedCount).toBeNull(); expect(result.risk).toBe('UNKNOWN'); expect(result.suggestion).toMatch(/context/); implSpy.mockRestore(); diff --git a/gitnexus/test/unit/cli-impact-pdg-format.test.ts b/gitnexus/test/unit/cli-impact-pdg-format.test.ts index cb17487a5..9e9f4870f 100644 --- a/gitnexus/test/unit/cli-impact-pdg-format.test.ts +++ b/gitnexus/test/unit/cli-impact-pdg-format.test.ts @@ -38,7 +38,7 @@ function pdgFindings(overrides: Record = {}): Record { // The PDG result family advertises a contract version (FIX #2) so external // MCP/agent consumers can version against future shape evolution. It is a // mode:'pdg'-only field — never on the default callgraph result. - expect(pdgFindings()).toMatchObject({ mode: 'pdg', pdgResultVersion: 2 }); + expect(pdgFindings()).toMatchObject({ mode: 'pdg', pdgResultVersion: 3 }); }); it('surfaces ambiguous-projection and unresolved block counts honestly', () => { diff --git a/gitnexus/test/unit/impact-pdg-compose-dedup.test.ts b/gitnexus/test/unit/impact-pdg-compose-dedup.test.ts index ecf1ce9e5..a3c92f4e7 100644 --- a/gitnexus/test/unit/impact-pdg-compose-dedup.test.ts +++ b/gitnexus/test/unit/impact-pdg-compose-dedup.test.ts @@ -36,7 +36,7 @@ const local = ( impactedCount: number, ): PdgImpactSuccessResult => ({ mode: 'pdg', - pdgResultVersion: 2, + pdgResultVersion: 3, target: { id: 'T', name: 'criterion', type: 'Function', filePath: 'src/a.ts' }, direction: 'downstream', risk: 'UNKNOWN',