mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-07 02:58:02 +00:00
fix(mcp): report UNKNOWN risk when an upstream impact walk finds no callers
`risk: LOW` asserts "safe to change" — a claim ABOUT callers. An upstream walk that resolved none has nothing to base it on: the symbol may be genuinely unused, or reached only through a reference class the index does not record (a property access on a plain object, a bare-identifier read of a module-scope const). Seeding LOW from an empty result is the false-safe signal `anyKnownRisk` already refuses to emit on the ambiguous-candidate path, and that #2687 removed by making an undetermined impactedCount `null` rather than `0`. Zero-caller upstream results now report risk UNKNOWN with a riskNote saying absence of edges is not evidence of disuse. Downstream is untouched: an empty downstream walk reports resolved callees, not safety. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
021ac30376
commit
8ddb9c1d2a
3 changed files with 139 additions and 3 deletions
|
|
@ -7050,8 +7050,27 @@ export class LocalBackend {
|
|||
// Risk scoring
|
||||
const processCount = affectedProcesses.length;
|
||||
const moduleCount = affectedModules.length;
|
||||
let risk = 'LOW';
|
||||
if (directCount >= 30 || processCount >= 5 || moduleCount >= 5 || impacted.length >= 200) {
|
||||
let risk: string;
|
||||
if (direction === 'upstream' && impacted.length === 0) {
|
||||
// An upstream walk that resolved NO callers cannot support `LOW`. "Safe
|
||||
// to change" is a claim ABOUT callers, and this walk found none to reason
|
||||
// about: the symbol may be genuinely unused, or reached only through a
|
||||
// reference class this index does not record — a property access on a
|
||||
// plain object, or a bare-identifier read of a module-scope `Const`,
|
||||
// neither of which mints a reference site today. Seeding `LOW` from an
|
||||
// empty result is the same false-safe signal `anyKnownRisk` refuses to
|
||||
// emit on the ambiguous-candidate path, and that #2687 removed by making
|
||||
// an undetermined `impactedCount` `null` instead of `0`.
|
||||
//
|
||||
// Downstream is deliberately untouched: an empty downstream walk reports
|
||||
// that this symbol resolved no callees, which is not a safety verdict.
|
||||
risk = 'UNKNOWN';
|
||||
} else if (
|
||||
directCount >= 30 ||
|
||||
processCount >= 5 ||
|
||||
moduleCount >= 5 ||
|
||||
impacted.length >= 200
|
||||
) {
|
||||
risk = 'CRITICAL';
|
||||
} else if (
|
||||
directCount >= 15 ||
|
||||
|
|
@ -7062,6 +7081,8 @@ export class LocalBackend {
|
|||
risk = 'HIGH';
|
||||
} else if (directCount >= 5 || impacted.length >= 30) {
|
||||
risk = 'MEDIUM';
|
||||
} else {
|
||||
risk = 'LOW';
|
||||
}
|
||||
|
||||
// Build per-depth counts (always included, even in summaryOnly mode)
|
||||
|
|
@ -7090,6 +7111,16 @@ export class LocalBackend {
|
|||
direction,
|
||||
impactedCount: impacted.length,
|
||||
risk,
|
||||
...(risk === 'UNKNOWN'
|
||||
? {
|
||||
riskNote:
|
||||
'No callers resolved. Absence of edges is not evidence the symbol is unused: ' +
|
||||
'a caller reaching it through a reference class this index does not record — ' +
|
||||
'plain-object property access, a bare-identifier read of a module-scope const — ' +
|
||||
'produces no edge to find. Confirm with a text search before treating the ' +
|
||||
'change as safe.',
|
||||
}
|
||||
: {}),
|
||||
...epistemic,
|
||||
...(!traversalComplete && { partial: true }),
|
||||
summary: {
|
||||
|
|
|
|||
|
|
@ -455,7 +455,8 @@ WHEN TO USE: Before making code changes — especially refactoring, renaming, or
|
|||
AFTER THIS: Review d=1 items (WILL BREAK). Use context() on high-risk symbols.
|
||||
|
||||
Output includes:
|
||||
- risk: LOW / MEDIUM / HIGH / CRITICAL / UNKNOWN
|
||||
- risk: LOW / MEDIUM / HIGH / CRITICAL / UNKNOWN. An upstream walk that resolved ZERO callers reports UNKNOWN, never LOW, and carries riskNote: "safe to change" is a claim about callers and there were none to reason about, so the symbol is either genuinely unused OR reached only through a reference class the index does not record (plain-object property access, a bare-identifier read of a module-scope const). Confirm with a text search before acting on it. Downstream walks are unaffected — an empty downstream result reports resolved callees, not safety.
|
||||
- riskNote: string — present only when risk is UNKNOWN; states why the verdict is withheld.
|
||||
- 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)
|
||||
|
|
|
|||
104
gitnexus/test/integration/impact-zero-caller-risk.test.ts
Normal file
104
gitnexus/test/integration/impact-zero-caller-risk.test.ts
Normal file
|
|
@ -0,0 +1,104 @@
|
|||
/**
|
||||
* Integration test: an empty upstream walk is UNKNOWN risk, never LOW.
|
||||
*
|
||||
* `risk: LOW` asserts "safe to change". That is a claim ABOUT callers, so a
|
||||
* walk that resolved NONE has nothing to base it on: the symbol is either
|
||||
* genuinely unused, or reached only through a reference class the index does
|
||||
* not record (a property access on a plain object, a bare-identifier read of a
|
||||
* module-scope `Const` — neither mints a reference site today). Reporting LOW
|
||||
* there is the false-safe signal `anyKnownRisk` already refuses to emit on the
|
||||
* ambiguous-candidate path, and that #2687 removed by making an undetermined
|
||||
* `impactedCount` `null` rather than `0`.
|
||||
*
|
||||
* Direction matters: an empty DOWNSTREAM walk says this symbol resolved no
|
||||
* callees, which is not a safety verdict, so it keeps its existing risk.
|
||||
*/
|
||||
import { it, expect, beforeAll, vi } from 'vitest';
|
||||
import { LocalBackend } from '../../src/mcp/local/local-backend.js';
|
||||
import { listRegisteredRepos } from '../../src/storage/repo-manager.js';
|
||||
import { withTestLbugDB } from '../helpers/test-indexed-db.js';
|
||||
|
||||
vi.mock('../../src/storage/repo-manager.js', () => ({
|
||||
listRegisteredRepos: vi.fn().mockResolvedValue([]),
|
||||
cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }),
|
||||
findSiblingClones: vi.fn().mockResolvedValue([]),
|
||||
}));
|
||||
|
||||
const SEED = [
|
||||
// No edges in either direction — the empty-walk case.
|
||||
`CREATE (orphan:Function {id: 'Function:src/orphan.ts:orphanHelper', name: 'orphanHelper', filePath: 'src/orphan.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`,
|
||||
// A resolved caller -> callee pair — the control that must stay LOW.
|
||||
`CREATE (used:Function {id: 'Function:src/used.ts:usedHelper', name: 'usedHelper', filePath: 'src/used.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`,
|
||||
`CREATE (caller:Function {id: 'Function:src/caller.ts:callerFn', name: 'callerFn', filePath: 'src/caller.ts', startLine: 1, endLine: 8, isExported: true, content: '', description: ''})`,
|
||||
`MATCH (a:Function {id:'Function:src/caller.ts:callerFn'}), (b:Function {id:'Function:src/used.ts:usedHelper'}) CREATE (a)-[:CodeRelation {type:'CALLS', confidence:0.9, reason:'direct', step:0}]->(b)`,
|
||||
];
|
||||
|
||||
withTestLbugDB(
|
||||
'impact-zero-caller-risk',
|
||||
(handle) => {
|
||||
let backend: LocalBackend;
|
||||
beforeAll(() => {
|
||||
backend = (handle as any)._backend;
|
||||
});
|
||||
|
||||
it('reports UNKNOWN, not LOW, when an upstream walk resolves no callers', async () => {
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'orphanHelper',
|
||||
direction: 'upstream',
|
||||
});
|
||||
expect(result).not.toHaveProperty('error');
|
||||
expect(result.impactedCount).toBe(0);
|
||||
expect(result.risk).toBe('UNKNOWN');
|
||||
});
|
||||
|
||||
it('explains the withheld verdict in riskNote', async () => {
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'orphanHelper',
|
||||
direction: 'upstream',
|
||||
});
|
||||
expect(typeof result.riskNote).toBe('string');
|
||||
// The note must say absence-of-edges is not proof of disuse; an agent
|
||||
// gating its own edits reads this instead of inferring safety from 0.
|
||||
expect(result.riskNote).toMatch(/not evidence/i);
|
||||
});
|
||||
|
||||
it('leaves a resolved caller set at LOW with no riskNote', async () => {
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'usedHelper',
|
||||
direction: 'upstream',
|
||||
});
|
||||
expect(result.impactedCount).toBeGreaterThanOrEqual(1);
|
||||
expect(result.risk).toBe('LOW');
|
||||
expect(result.riskNote).toBeUndefined();
|
||||
});
|
||||
|
||||
it('does not hedge an empty DOWNSTREAM walk — that is not a safety claim', async () => {
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'orphanHelper',
|
||||
direction: 'downstream',
|
||||
});
|
||||
expect(result.impactedCount).toBe(0);
|
||||
expect(result.risk).toBe('LOW');
|
||||
expect(result.riskNote).toBeUndefined();
|
||||
});
|
||||
},
|
||||
{
|
||||
seed: SEED,
|
||||
poolAdapter: true,
|
||||
afterSetup: async (handle) => {
|
||||
vi.mocked(listRegisteredRepos).mockResolvedValue([
|
||||
{
|
||||
name: 'test-repo',
|
||||
path: '/test/repo',
|
||||
storagePath: handle.tmpHandle.dbPath,
|
||||
indexedAt: new Date().toISOString(),
|
||||
lastCommit: 'abc123',
|
||||
stats: { files: 3, nodes: 3, communities: 0, processes: 0 },
|
||||
},
|
||||
]);
|
||||
const backend = new LocalBackend();
|
||||
await backend.init();
|
||||
(handle as any)._backend = backend;
|
||||
},
|
||||
},
|
||||
);
|
||||
Loading…
Add table
Reference in a new issue