mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-06 02:49:56 +00:00
fix(impact): harden PDG review findings
This commit is contained in:
parent
50586aa058
commit
84612056cc
5 changed files with 125 additions and 10 deletions
|
|
@ -559,6 +559,9 @@ async function run() {
|
|||
skipped: pdgRes.pdgLayer !== undefined && pdgRes.pdgLayer !== 'ready',
|
||||
};
|
||||
} finally {
|
||||
const lbugPath = path.join(work, '.gitnexus', 'lbug');
|
||||
await closeLbug(lbugPath).catch(() => {});
|
||||
initialised.delete(lbugPath);
|
||||
fs.rmSync(work, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -324,10 +324,17 @@ export function formatImpactResult(result: any): string {
|
|||
// No statement block at the line, or no dependents in this direction.
|
||||
// Print the honest note (pdg-no-block-at-line or the no-dependence note)
|
||||
// verbatim — never an empty "isolated" headline.
|
||||
return (
|
||||
`No statements ${direction}-dependent on ${anchorFile}:${result.criterionLine}.` +
|
||||
(result.note ? `\n${result.note}` : '')
|
||||
);
|
||||
const emptySliceLines = [
|
||||
`No statements ${direction}-dependent on ${anchorFile}:${result.criterionLine}.`,
|
||||
];
|
||||
if (result.truncated) {
|
||||
const by = formatTruncationSuffix(result);
|
||||
emptySliceLines.push(
|
||||
`⚠️ Truncated${by} — the dependence slice was bounded; deeper PDG-dependent statements may exist.`,
|
||||
);
|
||||
}
|
||||
if (result.note) emptySliceLines.push(result.note);
|
||||
return emptySliceLines.join('\n');
|
||||
}
|
||||
|
||||
const slLines: string[] = [];
|
||||
|
|
@ -587,7 +594,7 @@ function formatToolResult(toolName: string, result: any): string {
|
|||
// Guide the agent to the logical next tool call.
|
||||
// Critical for tool chaining: query → context → impact → fix.
|
||||
|
||||
function getNextStepHint(toolName: string): string {
|
||||
export function getNextStepHint(toolName: string, result?: any): string {
|
||||
switch (toolName) {
|
||||
case 'query':
|
||||
return '\n---\nNext: Pick a symbol above and run gitnexus-context "<name>" to see all its callers, callees, and execution flows.';
|
||||
|
|
@ -596,6 +603,15 @@ function getNextStepHint(toolName: string): string {
|
|||
return '\n---\nNext: To check what breaks if you change this, run gitnexus-impact "<name>" upstream';
|
||||
|
||||
case 'impact':
|
||||
if (
|
||||
result?.error ||
|
||||
result?.status === 'ambiguous' ||
|
||||
result?.mode === 'pdg' ||
|
||||
result?.pdgLayer ||
|
||||
typeof result?.criterionLine === 'number'
|
||||
) {
|
||||
return '';
|
||||
}
|
||||
return '\n---\nNext: Review d=1 items first (WILL BREAK). Read the source with cat to understand the code, then make your fix.';
|
||||
|
||||
case 'cypher':
|
||||
|
|
@ -711,7 +727,7 @@ export async function evalServerCommand(options?: EvalServerOptions): Promise<vo
|
|||
// Call tool, format result as text, append next-step hint
|
||||
const result = await backend.callTool(toolName, args);
|
||||
const formatted = formatToolResult(toolName, result);
|
||||
const hint = getNextStepHint(toolName);
|
||||
const hint = getNextStepHint(toolName, result);
|
||||
|
||||
res.setHeader('Content-Type', 'text/plain');
|
||||
res.writeHead(200);
|
||||
|
|
|
|||
|
|
@ -345,6 +345,8 @@ export type PdgDegradedLayerStatus = PdgLayerStatus & { state: PdgDegradedLayerS
|
|||
export interface PdgImpactDegradedResult extends PdgImpactBaseResult {
|
||||
pdgLayer: PdgDegradedLayerState;
|
||||
missingSubLayer?: PdgSubLayer;
|
||||
probeError?: string;
|
||||
recoverySuggestion?: string;
|
||||
}
|
||||
|
||||
export interface PdgImpactErrorResult {
|
||||
|
|
@ -398,6 +400,10 @@ export function makePdgLayerDegradedResult(input: {
|
|||
mode: input.mode,
|
||||
pdgLayer: input.layer.state,
|
||||
...(input.layer.missingSubLayer ? { missingSubLayer: input.layer.missingSubLayer } : {}),
|
||||
...(input.layer.probeError ? { probeError: input.layer.probeError } : {}),
|
||||
...(input.layer.recoverySuggestion
|
||||
? { recoverySuggestion: input.layer.recoverySuggestion }
|
||||
: {}),
|
||||
note: input.layer.note,
|
||||
target: input.target,
|
||||
direction: input.direction,
|
||||
|
|
@ -606,6 +612,10 @@ export interface PdgLayerStatus {
|
|||
missingSubLayer?: PdgSubLayer;
|
||||
/** Human-readable guidance for the degraded states (absent for `'ready'`). */
|
||||
note?: string;
|
||||
/** Set when an unknown-state probe failed before it could inspect PDG rows. */
|
||||
probeError?: string;
|
||||
/** Optional operator-facing recovery hint for probe failures. */
|
||||
recoverySuggestion?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -730,6 +740,7 @@ export async function pdgLayerStatus(deps: {
|
|||
// `'unknown'` (inconclusive), but the note distinguishes them so the operator
|
||||
// gets the more useful hint.
|
||||
let edgesVisible = false;
|
||||
let probeError: string | undefined;
|
||||
try {
|
||||
const rows = await deps.executeParameterized(
|
||||
deps.lbugPath,
|
||||
|
|
@ -737,11 +748,24 @@ export async function pdgLayerStatus(deps: {
|
|||
{},
|
||||
);
|
||||
edgesVisible = Array.isArray(rows) && rows.length > 0;
|
||||
} catch {
|
||||
} catch (err) {
|
||||
// db-lock / missing-path / corrupt probe — fall through as not-visible, but
|
||||
// keep the `'unknown'` signal (a probe failure must not lose it).
|
||||
// keep the `'unknown'` signal AND preserve the probe failure. Reporting the
|
||||
// failed probe as "no edges visible" hides a DB-health problem from operators.
|
||||
probeError = err instanceof Error ? err.message : String(err);
|
||||
edgesVisible = false;
|
||||
}
|
||||
if (probeError) {
|
||||
return {
|
||||
state: 'unknown',
|
||||
probeError,
|
||||
recoverySuggestion:
|
||||
'Check for a LadybugDB lock/corruption or missing index path. Stop overlapping GitNexus processes, retry, or re-run gitnexus analyze --pdg.',
|
||||
note:
|
||||
`PDG layer status unknown — CDG/REACHING_DEF probe failed: ${probeError}. ` +
|
||||
`The layer cannot be confirmed complete; this is distinct from "no edges visible".`,
|
||||
};
|
||||
}
|
||||
return {
|
||||
state: 'unknown',
|
||||
note: edgesVisible
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@
|
|||
* rendering stays byte-identical (regression guard).
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { formatImpactResult } from '../../src/cli/eval-server.js';
|
||||
import { formatImpactResult, getNextStepHint } from '../../src/cli/eval-server.js';
|
||||
|
||||
// A representative PDG findings result, shaped exactly like
|
||||
// `assemblePdgImpactResult` (pdg-impact.ts) emits.
|
||||
|
|
@ -370,6 +370,23 @@ describe('formatImpactResult — PDG (mode:pdg) rendering', () => {
|
|||
expect(out).toContain('by depth');
|
||||
});
|
||||
|
||||
it('flags truncated empty statement slices honestly', () => {
|
||||
const out = formatImpactResult(
|
||||
pdgStatementSlice({
|
||||
affectedStatements: [],
|
||||
affectedStatementCount: 0,
|
||||
truncated: true,
|
||||
truncatedBy: 'limit',
|
||||
note: 'Statement slice stopped at the configured result limit.',
|
||||
}),
|
||||
);
|
||||
|
||||
expect(out).toContain('No statements downstream-dependent on src/svc.ts:8');
|
||||
expect(out).toContain('Truncated');
|
||||
expect(out).toContain('by limit');
|
||||
expect(out).toContain('Statement slice stopped at the configured result limit.');
|
||||
});
|
||||
|
||||
it('renders a no-block-at-line result as the steering note, never an empty isolated headline', () => {
|
||||
// `_runImpactPDG` seedBlocks.length === 0 in statement mode.
|
||||
const out = formatImpactResult({
|
||||
|
|
@ -408,6 +425,14 @@ describe('formatImpactResult — PDG (mode:pdg) rendering', () => {
|
|||
expect(out).not.toContain('appears isolated');
|
||||
expect(out).not.toContain('PDG-dependent symbols');
|
||||
});
|
||||
|
||||
it('suppresses callgraph next-step hints for PDG and failed impact results', () => {
|
||||
expect(getNextStepHint('impact')).toContain('Review d=1 items first');
|
||||
expect(getNextStepHint('impact', pdgStatementSlice())).toBe('');
|
||||
expect(getNextStepHint('impact', { mode: 'pdg', pdgLayer: 'no-layer' })).toBe('');
|
||||
expect(getNextStepHint('impact', { mode: 'pdg', status: 'ambiguous' })).toBe('');
|
||||
expect(getNextStepHint('impact', { error: 'Target not found' })).toBe('');
|
||||
});
|
||||
});
|
||||
|
||||
describe('formatImpactResult — callgraph rendering is UNCHANGED (regression guard)', () => {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
import { IMPACT_MAX_DEPTH } from '../../src/mcp/tools.js';
|
||||
import { runImpactPDG } from '../../src/mcp/local/pdg-impact.js';
|
||||
import { pdgLayerStatus, runImpactPDG } from '../../src/mcp/local/pdg-impact.js';
|
||||
|
||||
describe('runImpactPDG', () => {
|
||||
it('clamps huge maxDepth values to the documented impact traversal cap', async () => {
|
||||
|
|
@ -74,3 +74,50 @@ describe('runImpactPDG', () => {
|
|||
expect((result as any).affectedStatements.map((s: any) => s.text)).toEqual(['a();', 'b();']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('pdgLayerStatus', () => {
|
||||
const unreadableMeta = async () => null as any;
|
||||
|
||||
it('reports visible PDG edges as unknown without a probe error when meta is unreadable', async () => {
|
||||
const result = await pdgLayerStatus({
|
||||
lbugPath: 'repo/.gitnexus/lbug',
|
||||
loadMetaFn: unreadableMeta,
|
||||
executeParameterized: (async (_repo: string, query: string) => {
|
||||
expect(query).toContain('LIMIT 1');
|
||||
return [{ type: 'CDG' }];
|
||||
}) as any,
|
||||
});
|
||||
|
||||
expect(result.state).toBe('unknown');
|
||||
expect(result.note).toContain('edges ARE visible');
|
||||
expect(result.probeError).toBeUndefined();
|
||||
});
|
||||
|
||||
it('reports no visible PDG edges separately from probe failures', async () => {
|
||||
const result = await pdgLayerStatus({
|
||||
lbugPath: 'repo/.gitnexus/lbug',
|
||||
loadMetaFn: unreadableMeta,
|
||||
executeParameterized: (async () => []) as any,
|
||||
});
|
||||
|
||||
expect(result.state).toBe('unknown');
|
||||
expect(result.note).toContain('no CDG/REACHING_DEF edges visible');
|
||||
expect(result.probeError).toBeUndefined();
|
||||
});
|
||||
|
||||
it('preserves probe failures instead of reporting a false no-edge signal', async () => {
|
||||
const result = await pdgLayerStatus({
|
||||
lbugPath: 'repo/.gitnexus/lbug',
|
||||
loadMetaFn: unreadableMeta,
|
||||
executeParameterized: (async () => {
|
||||
throw new Error('database busy');
|
||||
}) as any,
|
||||
});
|
||||
|
||||
expect(result.state).toBe('unknown');
|
||||
expect(result.probeError).toBe('database busy');
|
||||
expect(result.note).toContain('probe failed');
|
||||
expect(result.note).not.toContain('no CDG/REACHING_DEF edges visible');
|
||||
expect(result.recoverySuggestion).toContain('LadybugDB');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue