diff --git a/.claude/skills/gitnexus-impact-analysis/SKILL.md b/.claude/skills/gitnexus-impact-analysis/SKILL.md index 4fb73f3e6..85d90c90d 100644 --- a/.claude/skills/gitnexus-impact-analysis/SKILL.md +++ b/.claude/skills/gitnexus-impact-analysis/SKILL.md @@ -93,6 +93,15 @@ dispatch, cross-language calls), so few-callers ⇒ LOW does **not** apply. The result carries a `riskNote` saying so. Confirm with a text search before treating the symbol as safe to change or delete. +`risk` is the edit gate: warn on HIGH/CRITICAL and stop on UNKNOWN until the +uncertainty is resolved. Within single-repo mode, compare File and symbol +targets with local `riskSharedAxes` (direct/total only). Within group mode, +compare only group results: their `riskSharedAxes` overlays resolved +cross-repo crossings on that local value. Never use either field to waive the +edit gate. Check `riskScale.unusedAxes` before comparing kinds: MCP File walks +omit process/module axes, while web Graph-RAG expands File targets to in-file +symbols before enrichment. + ## Tools **impact** — the primary tool for symbol blast radius. If MCP is unavailable, use `node .gitnexus/run.cjs impact --direction upstream --repo .` instead: diff --git a/AGENTS.md b/AGENTS.md index c83a2e909..05be52af2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -119,10 +119,10 @@ This project is indexed by GitNexus as **GitNexus** (248612 symbols, 565510 rela - **MUST run impact analysis before editing.** Use `impact({target: "symbolName", direction: "upstream"})` (MCP) or `node .gitnexus/run.cjs impact "symbolName" --direction upstream --repo .` (CLI fallback); report callers, processes, and risk. Never substitute grep for graph analysis. For unified PDG impact, add `mode: "pdg"` with optional `line: ` — it returns statement-level `affectedStatements` over CDG + REACHING_DEF and inter-procedural symbols in `interproceduralByDepth`/`byDepth`; no-layer/degraded PDG results are UNKNOWN-risk notes (`--pdg` layer). CLI equivalent: `node .gitnexus/run.cjs impact "symbolName" --direction upstream --mode pdg --line --repo .`. - **MUST analyze graph changes before committing.** Use `detect_changes({scope: "all"})` (MCP) or `node .gitnexus/run.cjs detect-changes --scope all --repo .` (CLI fallback). `partial: true` or `truncated: true` is not a clean check — a zero means unseen, not unaffected; re-run it. For regression review: `detect_changes({scope: "compare", base_ref: "main"})` or `node .gitnexus/run.cjs detect-changes --scope compare --base-ref "main" --repo .`. -- **MUST warn the user** if impact analysis returns HIGH or CRITICAL risk before proceeding with edits. +- MUST warn on HIGH/CRITICAL `risk` pre-edit; never use `riskSharedAxes` to waive a HIGH/CRITICAL `risk` warning. Compare File/symbol: MCP File omits axes; Graph-RAG expands File. - **MUST treat `risk: UNKNOWN` as unresolved, not as low.** An empty caller set is not evidence the symbol is unused — it can also mean the callers are not resolvable by the index (plain-object property access, dynamic dispatch, cross-language calls). `impact` pairs `UNKNOWN` with a `riskNote` saying so. Confirm with a text search before treating the symbol as safe to change or delete; do not proceed on the strength of a zero. -- When exploring unfamiliar code, use `query({search_query: "concept"})` to find execution flows instead of grepping. It returns process-grouped results ranked by relevance. -- When you need full context on a specific symbol — callers, callees, which execution flows it participates in — use `context({name: "symbolName"})`. +- Explore with `query({search_query: "concept"})` for process-grouped flows. +- Use `context({name: "symbolName"})` for callers, callees, and flows. - For security review, `explain({target: "fileOrSymbol"})` lists taint findings (source→sink flows; needs `analyze --pdg`). - For control/data dependence, `pdg_query({mode: "controls", target: "fileOrSymbol"})` answers "under what condition does X run?" (CDG, incl. guard clauses) and `pdg_query({mode: "flows", target, variable})` traces "where does variable Y flow?" (REACHING_DEF). `--pdg` layer. diff --git a/CLAUDE.md b/CLAUDE.md index 55b84d583..10681d819 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -70,10 +70,10 @@ This project is indexed by GitNexus as **GitNexus** (248612 symbols, 565510 rela - **MUST run impact analysis before editing.** Use `impact({target: "symbolName", direction: "upstream"})` (MCP) or `node .gitnexus/run.cjs impact "symbolName" --direction upstream --repo .` (CLI fallback); report callers, processes, and risk. Never substitute grep for graph analysis. For unified PDG impact, add `mode: "pdg"` with optional `line: ` — it returns statement-level `affectedStatements` over CDG + REACHING_DEF and inter-procedural symbols in `interproceduralByDepth`/`byDepth`; no-layer/degraded PDG results are UNKNOWN-risk notes (`--pdg` layer). CLI equivalent: `node .gitnexus/run.cjs impact "symbolName" --direction upstream --mode pdg --line --repo .`. - **MUST analyze graph changes before committing.** Use `detect_changes({scope: "all"})` (MCP) or `node .gitnexus/run.cjs detect-changes --scope all --repo .` (CLI fallback). `partial: true` or `truncated: true` is not a clean check — a zero means unseen, not unaffected; re-run it. For regression review: `detect_changes({scope: "compare", base_ref: "main"})` or `node .gitnexus/run.cjs detect-changes --scope compare --base-ref "main" --repo .`. -- **MUST warn the user** if impact analysis returns HIGH or CRITICAL risk before proceeding with edits. +- MUST warn on HIGH/CRITICAL `risk` pre-edit; never use `riskSharedAxes` to waive a HIGH/CRITICAL `risk` warning. Compare File/symbol: MCP File omits axes; Graph-RAG expands File. - **MUST treat `risk: UNKNOWN` as unresolved, not as low.** An empty caller set is not evidence the symbol is unused — it can also mean the callers are not resolvable by the index (plain-object property access, dynamic dispatch, cross-language calls). `impact` pairs `UNKNOWN` with a `riskNote` saying so. Confirm with a text search before treating the symbol as safe to change or delete; do not proceed on the strength of a zero. -- When exploring unfamiliar code, use `query({search_query: "concept"})` to find execution flows instead of grepping. It returns process-grouped results ranked by relevance. -- When you need full context on a specific symbol — callers, callees, which execution flows it participates in — use `context({name: "symbolName"})`. +- Explore with `query({search_query: "concept"})` for process-grouped flows. +- Use `context({name: "symbolName"})` for callers, callees, and flows. - For security review, `explain({target: "fileOrSymbol"})` lists taint findings (source→sink flows; needs `analyze --pdg`). - For control/data dependence, `pdg_query({mode: "controls", target: "fileOrSymbol"})` answers "under what condition does X run?" (CDG, incl. guard clauses) and `pdg_query({mode: "flows", target, variable})` traces "where does variable Y flow?" (REACHING_DEF). `--pdg` layer. diff --git a/docs/plans/2026-08-28-gitnexus-plan-impact-file-risk.md b/docs/plans/2026-08-28-gitnexus-plan-impact-file-risk.md new file mode 100644 index 000000000..7e14f2174 --- /dev/null +++ b/docs/plans/2026-08-28-gitnexus-plan-impact-file-risk.md @@ -0,0 +1,312 @@ +# GitNexus Engineering Plan + +> Task: Fix #3075 — File `impact` risk is not comparable to Function/Method risk. +> Evidence verified at commit `6bff33d14cbfe1e7b4f04bca51507e9f64ef579c` (`feat/kotlin-const-resolver`); GitNexus index 129 commits behind, refresh skipped: full-repo `--index-only --pdg` rebuild is impractical this session. Scorer and schema claims are `[verified]` from source; live inversion numbers are `[graph]` on the stale index. + +## 1. Objective + +Make File vs symbol `impact.risk` honest for consumers: either they can tell the scales differ, or they can compare on a shared two-axis score. Do **not** DEFINES-bridge processes/modules onto File targets (issue reporter sampled 8/10 one-importer files jumping to HIGH/CRITICAL). Do **not** retune Function HIGH/CRITICAL thresholds (agent warn-before-edit). + +Acceptance: + +- A File with a wider blast radius than a Function in the same file no longer looks “safer” when a consumer only reads `risk`, **or** the result states that `risk` is not comparable across kinds and offers `riskSharedAxes` for comparison. +- File targets still cannot trip HIGH/CRITICAL via `processes_affected` / `modules_affected` unless those axes become real in the index (they are not today). +- Existing Function/Method labels under the current four-axis ladder stay the same for the same inputs. +- MCP `riskNote` remains UNKNOWN-only (`tools.ts` contract). + +## 2. Current Behaviour + +Callgraph `impact` ends in `LocalBackend._runImpactBFS` (`gitnexus/src/mcp/local/local-backend.ts`). After BFS it enriches impacted ids with `STEP_IN_PROCESS` and `MEMBER_OF`, then scores: + +```7720:7738:gitnexus/src/mcp/local/local-backend.ts + } else if ( + directCount >= 30 || + processCount >= 5 || + moduleCount >= 5 || + impacted.length >= 200 + ) { + risk = 'CRITICAL'; + } else if ( + directCount >= 15 || + processCount >= 3 || + moduleCount >= 3 || + impacted.length >= 100 + ) { + risk = 'HIGH'; + } else if (directCount >= 5 || impacted.length >= 30) { + risk = 'MEDIUM'; + } else { + risk = 'LOW'; + } +``` + +Empty upstream → `UNKNOWN` + `riskNote`. Downstream empty stays LOW. `skipEnrichment` (ambiguous probes) already scores on direct+total only. PDG mode forces `risk: UNKNOWN` (`composeUnifiedPdgImpactResult`) — out of scope. + +File BFS walk is mostly File←IMPORTS File. Enrichment queries those File ids. Processes are CALLS traces (`process-processor.ts`); communities admit only Function/Class/Method/Interface (`isCommunitySymbol` in `community-processor.ts:412-416`). File is not in that set. `enrichCandidateLabels` UNION also **omits File**, so File `target.type` is often `""`; detect File via `id` prefix `File:`. + +Web Graph RAG (`gitnexus-web/src/core/llm/tools.ts` ~1331–1346) duplicates the same ladder. + +## 3. Relevant Architecture + +| Layer | Role | +|---|---| +| Index | File never sources `STEP_IN_PROCESS` / `MEMBER_OF` by construction | +| MCP `_runImpactBFS` | Blast radius + four-axis `risk` | +| Ambiguous probes | `skipEnrichment` → 2-axis `risk` already | +| `mergeRisk` | Group overlay; monotone in crossings; does not know target kind | +| CLI `formatImpactResult` | Prints counts; **does not print `risk`** on the resolved callgraph path; JSON `impactCommand` still ships `risk` | +| `ai-context.ts` / `tools.ts` | Agent contract: warn on HIGH/CRITICAL; `riskNote` UNKNOWN-only | +| Web LLM `impact` | Same formula, prose `RISK:` line | + +Modules: Local (MCP), Cli (format/docs), Group (`mergeRisk`), gitnexus-web LLM tools. Shared package `gitnexus-shared` is already a dependency of both CLI and web. + +## 4. GitNexus Findings + +- Primary: `_runImpactBFS` — d=1 `[graph]` `impact(target:_runImpactBFS, maxDepth:1, includeTests:true)`: `_impactImpl`, `impactByUid`. Production chain `[verified]`: `impact` → `_impactImpl` → `_runImpactBFS`; `impactByUid` skips per-symbol process lists but **not** aggregation (`skipPerSymbolEnrichment` only). +- `LocalBackend.impact` d=1 `[graph]` `context`: `callTool`. +- Duplicate scorer `[verified]` grep: `gitnexus-web/src/core/llm/tools.ts`. +- `mergeRisk` `[verified]` callers in `src/`: only `runGroupImpact` (`cross-impact.ts:907`). Graph d=1 listed a test File (`impact-pdg-shape.test.ts`) and missed `runGroupImpact` — trust source. +- Schema `[verified]`: `isCommunitySymbol` excludes File; `schema.ts` documents MEMBER_OF as Function/Class/Method/Interface only. +- Live inversion `[graph]` stale index, `impact summaryOnly` on GitNexus: + +| target | kind | impacted | direct | processes | modules | risk | +|---|---|---|---|---|---|---| +| `lbug-config.ts` | File | 54 | 12 | 0 | 0 | MEDIUM | +| `openLbugConnection` | Function | 16 | 9 | 3 | 2 | HIGH | +| `local-backend.ts` | File | 12 | 10 | 0 | 0 | MEDIUM | +| `refreshRepos` | Method | 50 | 5 | 4 | 7 | CRITICAL | + +- Clusters/processes resources `[graph]`: Local/Cli/Group sit in the impact path; process traces are function-stepped, not File-stepped. +- Related tests `[verified]`: `test/unit/impact-pagination.test.ts` (CRITICAL from `direct=400`); `test/integration/impact-zero-caller-risk.test.ts` (`withTestLbugDB` seed — pattern to extend); `test/unit/eval-formatters.test.ts` (`formatImpactResult`); group `mergeRisk` tests. + +## 5. Statement-Level PDG Findings + +PDG unavailable (`pdg_query` on `_runImpactBFS`: “no PDG layer”). Recommend `node .gitnexus/run.cjs analyze --index-only --pdg` before any future statement-slice work. Control flow of the scorer is a straight if/else after enrichment; no hidden guards. `skipEnrichment` is the only branch that structurally zeros process/module counts besides File ids. + +## 6. Proposed Changes + +### 6.1 Extract `scoreImpactRisk` — `gitnexus-shared/src/impact-risk.ts` (new) + +- **Responsibility:** Pure function: `{ direction, directCount, processCount, moduleCount, impactedCount, unusedAxes }` → `{ risk, riskSharedAxes, riskScale }`. +- **Behaviour:** Existing UNKNOWN/CRITICAL/HIGH/MEDIUM/LOW thresholds unchanged when `unusedAxes` is empty. `riskSharedAxes` always scores as if `processCount=0` and `moduleCount=0` (UNKNOWN rule still applies). `riskScale.comparableAcrossKinds` is false iff `unusedAxes` is non-empty. `riskScale.unusedAxes` lists `{ axis, reason }`. +- **Constraints:** Zero deps. Export from `gitnexus-shared/src/index.ts`. Do not put MCP types here. +- **File detection:** caller passes unused axes; helper does not parse UIDs. + +### 6.2 Wire MCP — `_runImpactBFS` in `local-backend.ts` + +- After computing `processCount`/`moduleCount`, set `unusedAxes`: + - target `id` starts with `File:` **or** `symType === 'File'` → processes + modules, reason `file-nodes-have-no-process-or-community-membership`; + - `skipEnrichment` → same axes, reason `enrichment-skipped` (ambiguous probes). +- Replace inline ladder with `scoreImpactRisk`. +- Spread `riskScale` and `riskSharedAxes` on the result next to `risk`. Do **not** set `riskNote` for File. +- Ambiguous candidate summaries: forward the new fields (probes already skip enrichment). +- `target.type` for File: if still `""`, prefer `'File'` when `id` starts with `File:` (display-only; helps CLI). + +### 6.3 Web duplicate — `gitnexus-web/src/core/llm/tools.ts` + +- Import `scoreImpactRisk` from `gitnexus-shared`. Print `RISK:` from `risk`; if `!comparableAcrossKinds`, one extra line: not comparable to Function risk; shared-axes label is `riskSharedAxes`. + +### 6.4 Agent/MCP contract copy + +- `gitnexus/src/mcp/tools.ts` impact description: document `riskScale` / `riskSharedAxes`; keep `riskNote` UNKNOWN-only; say File `risk` is not comparable to symbol `risk`. +- `gitnexus/src/cli/ai-context.ts`: HIGH/CRITICAL warning still applies; add: do not rank a File `MEDIUM` below a contained Function `HIGH` without `riskSharedAxes`. +- `formatImpactResult`: on resolved callgraph results with `risk`, print `Risk: {risk}` and, when incomparable, `Shared-axes risk: {riskSharedAxes} (File/process axes unused)`. + +### 6.5 Explicitly not changing + +- DEFINES-bridge, community/process indexers, `mergeRisk` formula, PDG `UNKNOWN`, `detectChanges` `risk_level`, Function thresholds. + +## 7. Implementation Sequence + +1. Add `gitnexus-shared` helper + unit table (issue-shaped inputs + UNKNOWN + skipEnrichment). Shared package tests if present; otherwise `gitnexus/test/unit/impact-risk.test.ts` importing the helper. +2. Switch `_runImpactBFS` + candidate probe payload. Tree still coherent: old `risk` values identical for Function fixtures. +3. Integration seed in `impact-zero-caller-risk.test.ts` **or** new `impact-file-risk-scale.test.ts`: File with ≥5 File IMPORTS (MEDIUM on direct) vs Function with 3 process-member callers (HIGH); assert File `riskScale.comparableAcrossKinds === false`, Function true, File `riskSharedAxes === risk`, Function `riskSharedAxes` is LOW/MEDIUM while `risk` is HIGH. +4. CLI formatter + `eval-formatters.test.ts`. +5. `tools.ts` + `ai-context.ts` wording. +6. Web import + a unit assertion on the printed RISK block if a test already covers that tool. +7. `npx tsc --noEmit` in `gitnexus/` and `gitnexus-web/`; `cd gitnexus && npm run test:unit -- test/unit/impact-risk.test.ts test/unit/eval-formatters.test.ts`; integration file from step 3. + +## 8. Test Strategy + +| File | Scenarios | +|---|---| +| `gitnexus/test/unit/impact-risk.test.ts` (new) | Issue table: File(25,13,0,0)→MEDIUM; Function(15,2,4,2)→HIGH; shared-axes File MEDIUM vs Function LOW; empty upstream UNKNOWN; downstream empty LOW; skipEnrichment unused axes; CRITICAL via direct≥30 still works with unused process axes | +| `gitnexus/test/integration/impact-file-risk-scale.test.ts` (new) | `withTestLbugDB` seed: `File:src/crypto.ts` ← 13 File IMPORTS, no File STEP_IN_PROCESS; `getEncryptionKey` with 2 CALLS from functions that have STEP_IN_PROCESS to 4 distinct Process nodes — reproduce inversion; assert new fields | +| `gitnexus/test/integration/impact-zero-caller-risk.test.ts` | Unchanged UNKNOWN/`riskNote`; candidates may grow `riskScale` — assert still present only when UNKNOWN for `riskNote` | +| `gitnexus/test/unit/impact-pagination.test.ts` | Hub CRITICAL unchanged | +| `gitnexus/test/unit/eval-formatters.test.ts` | Resolved result prints Risk + shared-axes line for File-shaped `riskScale` | +| Web | Only if an existing Graph RAG impact test snapshots `RISK:` | + +Commands (exist in `gitnexus/package.json`): `npm run test:unit`, `npm test` (full vitest), `npx tsc --noEmit`. Web: `npm test`, `npx tsc -b --noEmit`. Integration needs `pretest:integration` / `npm run test:integration` (runs `scripts/build.js`). + +## 9. Risk and Impact Analysis + +Direct dependents of `_runImpactBFS` `[graph]`: `_impactImpl`, `impactByUid`. `_impactImpl` is the only d=1 of `impact` besides the method’s own class. Any JSON consumer of `impact` (MCP, CLI `output(result)`, group local leg) sees additive fields — compatible if they ignore unknowns. + +- **HIGH workflow:** Function HIGH/CRITICAL unchanged. File still cannot reach HIGH via processes; a File with `direct≥15` or `total≥100` still can. Agents that compare File MEDIUM vs Function HIGH must start using `riskSharedAxes` or `riskScale`. +- **Ambiguous `maxRisk`:** probes skip enrichment, so File vs Function candidates are already 2-axis there — inversion is weaker on that path. +- **Group `mergeRisk`:** still compares incomparable File local `risk` to crossing count. Do not retune this PR; if a group File target is common, follow-up. +- **Web:** browser bundle picks up `gitnexus-shared` export — confirm `gitnexus-shared` build/exports include the new file. +- **Performance:** none (pure arithmetic after existing enrichment). +- **Ladybug empty labels:** File detection must not rely on `symType` alone. + +## 10. Files Expected to Change + +| File | Symbols | Reason | +|---|---|---| +| `gitnexus-shared/src/impact-risk.ts` | `scoreImpactRisk` | New shared scorer | +| `gitnexus-shared/src/index.ts` | exports | Public helper | +| `gitnexus/src/mcp/local/local-backend.ts` | `_runImpactBFS`, ambiguous candidate map | Wire scorer + File unused axes | +| `gitnexus/src/mcp/tools.ts` | `impact` description | Contract | +| `gitnexus/src/cli/ai-context.ts` | generated Always Do | Agent warning | +| `gitnexus/src/cli/eval-server.ts` | `formatImpactResult` | Print scale | +| `gitnexus-web/src/core/llm/tools.ts` | web `impact` | Same formula | +| `gitnexus/test/unit/impact-risk.test.ts` | — | Table tests | +| `gitnexus/test/integration/impact-file-risk-scale.test.ts` | — | Seeded inversion | +| `gitnexus/test/unit/eval-formatters.test.ts` | `formatImpactResult` | Formatter | + +## 11. Reusable Implementation Context + +```yaml +implementation_context: + task_summary: "Fix #3075: File impact.risk is a 2-axis score silently labelled on a 4-axis scale. Extract scoreImpactRisk; mark File/skipEnrichment axes unused; add riskScale + riskSharedAxes; do not DEFINES-bridge or retune Function thresholds." + acceptance_criteria: + - "File vs Function comparison is either labelled incomparable (riskScale) or done via riskSharedAxes" + - "Function/Method risk for identical four-axis inputs unchanged" + - "riskNote still UNKNOWN-only" + - "Integration seed reproduces crypto.ts-style inversion and asserts the new fields" + primary_symbols: + - symbol: "_runImpactBFS" + file: "gitnexus/src/mcp/local/local-backend.ts" + lines: "6991-7888" + role: "BFS + enrichment + inline risk ladder (replace ladder only)" + - symbol: "scoreImpactRisk" + file: "gitnexus-shared/src/impact-risk.ts" + lines: "new" + role: "Pure scorer + shared-axes + riskScale" + - symbol: "formatImpactResult" + file: "gitnexus/src/cli/eval-server.ts" + lines: "305-641" + role: "Human/LLM text surface for impact JSON" + related_symbols: + - symbol: "_impactImpl" + relationship: "CALLS" + relevance: "Resolves target, PDG vs callgraph, ambiguous skipEnrichment probes" + - symbol: "impactByUid" + relationship: "CALLS" + relevance: "Group fan-out; keep skipPerSymbolEnrichment; still run aggregation" + - symbol: "mergeRisk" + relationship: "consumes risk string" + relevance: "Do not change this PR" + - symbol: "isCommunitySymbol" + relationship: "index gate" + relevance: "Why File modules_affected is always 0" + - symbol: "composeUnifiedPdgImpactResult" + relationship: "separate path" + relevance: "PDG risk stays UNKNOWN" + execution_path: + - "impact / callTool → _impactImpl (resolve symbol, File id prefix File:)" + - "_runImpactBFS: IMPORTS-heavy walk for File; CALLS walk for Function" + - "Enrich STEP_IN_PROCESS / MEMBER_OF on impacted ids (empty for File ids)" + - "scoreImpactRisk with unusedAxes for File or skipEnrichment" + - "JSON to MCP/CLI; formatImpactResult for eval text; web LLM tools parallel path" + pdg_constraints: + - description: "No PDG layer on the planning index; scorer is post-enrichment arithmetic" + affected_statements: [] + implementation_consequence: "Do not wait on PDG; do not change pdg impact risk" + architectural_patterns: + - pattern: "Additive optional JSON fields on impact (riskNote, epistemic, partial)" + example_location: "gitnexus/src/mcp/local/local-backend.ts _runImpactBFS base object ~7754" + usage_guidance: "Add riskScale/riskSharedAxes the same way; never overload riskNote" + - pattern: "withTestLbugDB CREATE seed for impact contract" + example_location: "gitnexus/test/integration/impact-zero-caller-risk.test.ts" + usage_guidance: "Seed File IMPORTS + Function CALLS + Process membership separately" + files_to_modify: + - file: "gitnexus-shared/src/impact-risk.ts" + symbols: ["scoreImpactRisk"] + intended_change: "new pure scorer" + - file: "gitnexus-shared/src/index.ts" + symbols: [] + intended_change: "re-export" + - file: "gitnexus/src/mcp/local/local-backend.ts" + symbols: ["_runImpactBFS"] + intended_change: "unusedAxes + helper; File type display" + - file: "gitnexus/src/mcp/tools.ts" + symbols: [] + intended_change: "document fields" + - file: "gitnexus/src/cli/ai-context.ts" + symbols: [] + intended_change: "agent comparability note" + - file: "gitnexus/src/cli/eval-server.ts" + symbols: ["formatImpactResult"] + intended_change: "print risk + shared-axes when incomparable" + - file: "gitnexus-web/src/core/llm/tools.ts" + symbols: [] + intended_change: "import helper; extra prose line" + tests: + - file: "gitnexus/test/unit/impact-risk.test.ts" + scenarios: + - "File(25,13,0,0)+unused process/module → risk MEDIUM, comparableAcrossKinds false, riskSharedAxes MEDIUM" + - "Function(15,2,4,2) → HIGH, riskSharedAxes LOW (direct 2, total 15)" + - "upstream impactedCount 0 → UNKNOWN both fields" + - "direct 400 → CRITICAL even with unused process axes" + - file: "gitnexus/test/integration/impact-file-risk-scale.test.ts" + scenarios: + - "Seed File crypto.ts with 13 File importers vs getEncryptionKey with process-rich callers → inversion on risk, File incomparable, Function comparable" + - file: "gitnexus/test/unit/eval-formatters.test.ts" + scenarios: + - "formatImpactResult includes Shared-axes risk when riskScale.comparableAcrossKinds is false" + verification_commands: + - "cd gitnexus && npx tsc --noEmit" + - "cd gitnexus && npm run test:unit -- test/unit/impact-risk.test.ts test/unit/eval-formatters.test.ts test/unit/impact-pagination.test.ts" + - "cd gitnexus && npm run test:integration -- test/integration/impact-file-risk-scale.test.ts test/integration/impact-zero-caller-risk.test.ts" + - "cd gitnexus-web && npx tsc -b --noEmit" + risks: + - "Consumers that only read risk still see the inversion unless they adopt riskScale/riskSharedAxes — that is the chosen (explicit-scale) fix" + - "File type often empty; must key unusedAxes off File: id prefix" + - "gitnexus-shared export must reach the web bundle" + assumptions: + - "WHAT: File nodes never gain STEP_IN_PROCESS/MEMBER_OF without an indexer change. HOW: keep isCommunitySymbol and process traces as-is; tests seed File with zero such edges" + - "WHAT: Additive JSON fields are backward compatible. HOW: existing tests that exact-match the full impact object may need to allow extra keys — grep expect(res).toEqual on impact results before landing" + - "WHAT: HEAD 6bff33d is the pin; scorer line numbers ~7720. HOW: re-read the ladder if that hunk moved" + open_questions: + - "Whether GroupImpactResult should copy riskScale from local File targets (deferred unless tests already snapshot the full group object)" + avoid: + - "Do not DEFINES-bridge File→symbol processes/modules" + - "Do not lower Function process/module HIGH/CRITICAL thresholds" + - "Do not reuse riskNote for File incomparability" + - "Do not change PDG impact risk or detectChanges risk_level" + - "Do not treat labels(n)[0] or empty target.type as proof the node is not a File" + - "Do not repeat full repository discovery" +``` + +## 12. Assumptions and Open Questions + +**Assumptions** + +- Indexer will not start attaching File→Process/Community in this change (`isCommunitySymbol` stays). `[verified]` source; `[assumed]` future indexers. +- Ignoring unknown JSON keys is safe for MCP clients; any `toEqual` goldens in-repo must be updated. `[assumed]` — grep during implement. +- Stale-index inversion (`lbug-config.ts` vs `openLbugConnection`) is illustrative; the integration seed is the regression lock. `[graph]` vs `[verified]` seed. + +**Open questions** + +- Group `mergeRisk` + File local risk: copy `riskScale` onto `GroupImpactResult`? Default **no** unless a test breaks. +- Class/Interface STEP_IN_PROCESS sparsity: out of scope (#3075 is File). +- Printing `risk` on CLI formatted output is new (JSON already has it). Keep the extra lines short. + +**Deferred** + +- Recalibrated File-only HIGH thresholds. +- Indexing File community membership. +- DEFINES-bridge after a threshold RFC. +- Related #2975 (docs vs scorer wording) except as touched by `tools.ts`. + +## 13. Definition of Done + +- [ ] `scoreImpactRisk` is the only callgraph ladder in MCP and web. +- [ ] File (and skipEnrichment) results include `riskScale.comparableAcrossKinds === false` and `riskSharedAxes`. +- [ ] Function four-axis HIGH/CRITICAL cases in unit tests still pass with the same labels. +- [ ] Integration seed proves wider File blast + lower `risk` than a contained Function, and `riskSharedAxes` orders them without pretending processes existed on the File. +- [ ] `riskNote` still absent unless `risk === 'UNKNOWN'`. +- [ ] `tools.ts` + `ai-context.ts` state that File `risk` is not comparable to symbol `risk`. +- [ ] `cd gitnexus && npx tsc --noEmit` and the named unit/integration commands pass; web typecheck passes. diff --git a/gitnexus-claude-plugin/skills/gitnexus-impact-analysis/SKILL.md b/gitnexus-claude-plugin/skills/gitnexus-impact-analysis/SKILL.md index 4fb73f3e6..85d90c90d 100644 --- a/gitnexus-claude-plugin/skills/gitnexus-impact-analysis/SKILL.md +++ b/gitnexus-claude-plugin/skills/gitnexus-impact-analysis/SKILL.md @@ -93,6 +93,15 @@ dispatch, cross-language calls), so few-callers ⇒ LOW does **not** apply. The result carries a `riskNote` saying so. Confirm with a text search before treating the symbol as safe to change or delete. +`risk` is the edit gate: warn on HIGH/CRITICAL and stop on UNKNOWN until the +uncertainty is resolved. Within single-repo mode, compare File and symbol +targets with local `riskSharedAxes` (direct/total only). Within group mode, +compare only group results: their `riskSharedAxes` overlays resolved +cross-repo crossings on that local value. Never use either field to waive the +edit gate. Check `riskScale.unusedAxes` before comparing kinds: MCP File walks +omit process/module axes, while web Graph-RAG expands File targets to in-file +symbols before enrichment. + ## Tools **impact** — the primary tool for symbol blast radius. If MCP is unavailable, use `node .gitnexus/run.cjs impact --direction upstream --repo .` instead: diff --git a/gitnexus-cursor-integration/skills/gitnexus-impact-analysis/SKILL.md b/gitnexus-cursor-integration/skills/gitnexus-impact-analysis/SKILL.md index 4fb73f3e6..85d90c90d 100644 --- a/gitnexus-cursor-integration/skills/gitnexus-impact-analysis/SKILL.md +++ b/gitnexus-cursor-integration/skills/gitnexus-impact-analysis/SKILL.md @@ -93,6 +93,15 @@ dispatch, cross-language calls), so few-callers ⇒ LOW does **not** apply. The result carries a `riskNote` saying so. Confirm with a text search before treating the symbol as safe to change or delete. +`risk` is the edit gate: warn on HIGH/CRITICAL and stop on UNKNOWN until the +uncertainty is resolved. Within single-repo mode, compare File and symbol +targets with local `riskSharedAxes` (direct/total only). Within group mode, +compare only group results: their `riskSharedAxes` overlays resolved +cross-repo crossings on that local value. Never use either field to waive the +edit gate. Check `riskScale.unusedAxes` before comparing kinds: MCP File walks +omit process/module axes, while web Graph-RAG expands File targets to in-file +symbols before enrichment. + ## Tools **impact** — the primary tool for symbol blast radius. If MCP is unavailable, use `node .gitnexus/run.cjs impact --direction upstream --repo .` instead: diff --git a/gitnexus-shared/src/impact-risk.ts b/gitnexus-shared/src/impact-risk.ts index 6c18614f6..413d02f76 100644 --- a/gitnexus-shared/src/impact-risk.ts +++ b/gitnexus-shared/src/impact-risk.ts @@ -6,6 +6,7 @@ export type UnusedImpactRiskReason = | 'file-nodes-have-no-process-or-community-membership' | 'enrichment-skipped' | 'enrichment-budget-exhausted' + | 'enrichment-truncated' | 'enrichment-query-failed'; export interface UnusedImpactRiskAxis { @@ -50,7 +51,20 @@ function score( return 'LOW'; } -function countsWithUnusedAxesZeroed( +const UNMEASURED_REASONS: ReadonlySet = new Set([ + 'file-nodes-have-no-process-or-community-membership', + 'enrichment-skipped', + 'enrichment-budget-exhausted', +]); + +function unusedPair(reason: UnusedImpactRiskReason): UnusedImpactRiskAxis[] { + return [ + { axis: 'processes', reason }, + { axis: 'modules', reason }, + ]; +} + +function countsWithUnmeasuredAxesZeroed( input: ImpactRiskInput, ): Pick< ImpactRiskInput, @@ -59,6 +73,7 @@ function countsWithUnusedAxesZeroed( let processCount = input.processCount; let moduleCount = input.moduleCount; for (const unused of input.unusedAxes ?? []) { + if (!UNMEASURED_REASONS.has(unused.reason)) continue; if (unused.axis === 'processes') processCount = 0; if (unused.axis === 'modules') moduleCount = 0; } @@ -79,33 +94,23 @@ export function unusedAxesForImpactWalk(input: { processQueryFailed: boolean; moduleQueryFailed: boolean; /** When 0, a zero chunk budget is not an unused-axis event — there was nothing to enrich. */ - impactedCount?: number; + impactedCount: number; + /** True when process/module queries ran on a strict subset of impacted symbols. */ + enrichmentTruncated?: boolean; }): UnusedImpactRiskAxis[] { if (input.isFileTarget) { - return [ - { - axis: 'processes', - reason: 'file-nodes-have-no-process-or-community-membership', - }, - { - axis: 'modules', - reason: 'file-nodes-have-no-process-or-community-membership', - }, - ]; + return unusedPair('file-nodes-have-no-process-or-community-membership'); } if (input.skipEnrichment) { - return [ - { axis: 'processes', reason: 'enrichment-skipped' }, - { axis: 'modules', reason: 'enrichment-skipped' }, - ]; + return unusedPair('enrichment-skipped'); } - if (input.maxChunks === 0 && (input.impactedCount ?? 1) > 0) { - return [ - { axis: 'processes', reason: 'enrichment-budget-exhausted' }, - { axis: 'modules', reason: 'enrichment-budget-exhausted' }, - ]; + if (input.maxChunks === 0 && input.impactedCount > 0) { + return unusedPair('enrichment-budget-exhausted'); } const unused: UnusedImpactRiskAxis[] = []; + if (input.enrichmentTruncated) { + unused.push(...unusedPair('enrichment-truncated')); + } if (input.processQueryFailed) { unused.push({ axis: 'processes', reason: 'enrichment-query-failed' }); } @@ -115,11 +120,28 @@ export function unusedAxesForImpactWalk(input: { return unused; } +const INCOMPLETE_SAMPLE_REASONS: ReadonlySet = new Set([ + 'enrichment-query-failed', + 'enrichment-truncated', +]); + export function scoreImpactRisk(input: ImpactRiskInput): ImpactRiskResult { const unusedAxes = input.unusedAxes ?? []; + const observedRisk = score(countsWithUnmeasuredAxesZeroed(input)); + const incompleteSample = unusedAxes.some((unused) => + INCOMPLETE_SAMPLE_REASONS.has(unused.reason), + ); + // Failed queries and truncated samples make observed process/module counts + // lower bounds. Preserve any HIGH/CRITICAL warning already proved by those + // counts, but never emit a confident LOW/MEDIUM edit gate from an incomplete + // enrichment pass. + const risk = + incompleteSample && (observedRisk === 'LOW' || observedRisk === 'MEDIUM') + ? 'UNKNOWN' + : observedRisk; return { - risk: score(countsWithUnusedAxesZeroed(input)), + risk, riskSharedAxes: score({ ...input, processCount: 0, moduleCount: 0 }), riskScale: { comparableAcrossKinds: unusedAxes.length === 0, diff --git a/gitnexus-web/src/core/llm/tools.ts b/gitnexus-web/src/core/llm/tools.ts index a702e7db8..407018678 100644 --- a/gitnexus-web/src/core/llm/tools.ts +++ b/gitnexus-web/src/core/llm/tools.ts @@ -13,7 +13,7 @@ import { tool } from '@langchain/core/tools'; import { z } from 'zod'; -import { NODE_TABLES, REL_TYPES } from 'gitnexus-shared'; +import { NODE_TABLES, REL_TYPES, scoreImpactRisk, unusedAxesForImpactWalk } from 'gitnexus-shared'; import type { EnrichedSearchResult, GrepResult } from '../../services/backend-client'; /** @@ -1275,6 +1275,9 @@ MATCH (n:Function {id: emb.nodeId}) RETURN n`, stepCount: number | null; }> = []; let affectedClusters: Array<{ label: string; hits: number; impact: string }> = []; + let processQueryFailed = false; + let clusterQueryFailed = false; + let clusterClassificationFailed = false; if (trimmedIds.length > 0) { const processQuery = ` @@ -1302,9 +1305,23 @@ MATCH (n:Function {id: emb.nodeId}) RETURN n`, : ''; const [processRes, clusterRes, directClusterRes] = await Promise.all([ - executeQuery(processQuery), - executeQuery(clusterQuery), - directClusterQuery ? executeQuery(directClusterQuery) : Promise.resolve([]), + executeQuery(processQuery).catch((err) => { + processQueryFailed = true; + if (import.meta.env.DEV) console.warn('Impact process enrichment failed:', err); + return []; + }), + executeQuery(clusterQuery).catch((err) => { + clusterQueryFailed = true; + if (import.meta.env.DEV) console.warn('Impact cluster enrichment failed:', err); + return []; + }), + directClusterQuery + ? executeQuery(directClusterQuery).catch((err) => { + clusterClassificationFailed = true; + if (import.meta.env.DEV) console.warn('Impact cluster enrichment failed:', err); + return []; + }) + : Promise.resolve([]), ]); const directClusterSet = new Set(); @@ -1323,7 +1340,11 @@ MATCH (n:Function {id: emb.nodeId}) RETURN n`, affectedClusters = clusterRes.map((row: any) => { const label = Array.isArray(row) ? row[0] : row.label; const hits = Array.isArray(row) ? row[1] : row.hits; - const impact = directClusterSet.has(label) ? 'direct' : 'indirect'; + const impact = clusterClassificationFailed + ? 'classification-unavailable' + : directClusterSet.has(label) + ? 'direct' + : 'indirect'; return { label, hits, impact }; }); } @@ -1331,19 +1352,25 @@ MATCH (n:Function {id: emb.nodeId}) RETURN n`, const directCount = depth1.length; const processCount = affectedProcesses.length; const clusterCount = affectedClusters.length; - let risk = 'LOW'; - if (directCount >= 30 || processCount >= 5 || clusterCount >= 5 || totalAffected >= 200) { - risk = 'CRITICAL'; - } else if ( - directCount >= 15 || - processCount >= 3 || - clusterCount >= 3 || - totalAffected >= 100 - ) { - risk = 'HIGH'; - } else if (directCount >= 5 || totalAffected >= 30) { - risk = 'MEDIUM'; - } + const enrichmentCapped = allNodeIds.length > maxIdsForContext; + const unusedAxes = unusedAxesForImpactWalk({ + isFileTarget: false, + skipEnrichment: false, + maxChunks: 10, + processQueryFailed, + moduleQueryFailed: clusterQueryFailed, + impactedCount: totalAffected, + enrichmentTruncated: enrichmentCapped, + }); + const scored = scoreImpactRisk({ + direction, + directCount, + processCount, + moduleCount: clusterCount, + impactedCount: totalAffected, + unusedAxes, + }); + const { risk, riskSharedAxes, riskScale } = scored; // ===== COMPACT TABULAR OUTPUT ===== const lines: string[] = [ @@ -1351,22 +1378,42 @@ MATCH (n:Function {id: emb.nodeId}) RETURN n`, `Confidence: High ${confidenceBuckets.high} | Medium ${confidenceBuckets.medium} | Low ${confidenceBuckets.low}`, ``, `AFFECTED PROCESSES:`, - ...(affectedProcesses.length > 0 - ? affectedProcesses.map( - (p) => - `- ${p.label} - BROKEN at step ${p.minStep ?? '?'} (${p.hits} symbols, ${p.stepCount ?? '?'} steps)`, - ) - : ['- None found']), + ...(processQueryFailed + ? ['- Unavailable (enrichment query failed)'] + : affectedProcesses.length > 0 + ? affectedProcesses.map( + (p) => + `- ${p.label} - BROKEN at step ${p.minStep ?? '?'} (${p.hits} symbols, ${p.stepCount ?? '?'} steps)`, + ) + : ['- None found']), ``, `AFFECTED CLUSTERS:`, - ...(affectedClusters.length > 0 - ? affectedClusters.map((c) => `- ${c.label} (${c.impact}, ${c.hits} symbols)`) - : ['- None found']), + ...(clusterQueryFailed + ? ['- Unavailable (enrichment query failed)'] + : affectedClusters.length > 0 + ? affectedClusters.map((c) => `- ${c.label} (${c.impact}, ${c.hits} symbols)`) + : ['- None found']), ``, - `RISK: ${risk}`, + `RISK: ${risk} (edit gate — warn on HIGH/CRITICAL)`, + `Shared-axes: ${riskSharedAxes} (File vs symbol compare only; do not waive a HIGH risk warning)`, + `Note: this Graph-RAG surface expands File targets to in-file symbols before enrichment, so process/cluster axes are comparable here when enrichment succeeds. MCP File impact does not.`, + ...(riskScale.comparableAcrossKinds + ? [] + : [ + `Note: process/module axes were unused (${riskScale.unusedAxes.map((a) => a.reason).join(', ')}).`, + ]), + ...(risk === 'UNKNOWN' && (processQueryFailed || clusterQueryFailed) + ? ['Note: risk is unresolved because enrichment failed; retry before editing.'] + : []), + ...(enrichmentCapped + ? [`Note: process/cluster enrichment is partial (first ${maxIdsForContext} symbols).`] + : []), + ...(clusterClassificationFailed + ? ['Note: direct/indirect cluster classification is unavailable.'] + : []), `- Direct callers: ${directCount}`, - `- Processes affected: ${processCount}`, - `- Clusters affected: ${clusterCount}`, + `- Processes affected: ${processQueryFailed ? 'unavailable' : processCount}`, + `- Clusters affected: ${clusterQueryFailed ? 'unavailable' : clusterCount}`, ``, ]; @@ -1472,7 +1519,9 @@ relationTypes filter (optional): Additional output sections: - Affected processes (with step impact) - Affected clusters (direct/indirect) -- Risk summary (based on direct callers, processes, clusters)`, +- RISK is the edit gate: warn before edits on HIGH/CRITICAL; UNKNOWN requires retry or corroboration +- Shared-axes risk compares File and symbol targets using direct/total counts only; it never waives the RISK gate +- riskScale notes unavailable process/module axes. This Graph-RAG tool expands File targets to in-file symbols; MCP File impact does not`, schema: z.object({ target: z.string().describe('Name of the function, class, or file to analyze'), direction: z diff --git a/gitnexus-web/test/unit/impact-tool.test.ts b/gitnexus-web/test/unit/impact-tool.test.ts new file mode 100644 index 000000000..f04817ed1 --- /dev/null +++ b/gitnexus-web/test/unit/impact-tool.test.ts @@ -0,0 +1,209 @@ +import { describe, expect, it, vi } from 'vitest'; +import { createGraphRAGTools, type GraphRAGBackend } from '../../src/core/llm/tools'; + +const noOpBackend: GraphRAGBackend = { + executeQuery: async () => [], + search: async () => [], + grep: async () => [], + readFile: async () => '', +}; + +function impactTool(backend: GraphRAGBackend) { + return createGraphRAGTools(backend).find((candidate) => candidate.name === 'impact')!; +} + +describe('Graph-RAG impact risk contract', () => { + it('advertises the edit gate, shared axes, and MCP File difference', () => { + const description = impactTool(noOpBackend).description; + expect(description).toContain('RISK is the edit gate'); + expect(description).toContain('Shared-axes risk'); + expect(description).toContain('riskScale'); + expect(description).toContain('MCP File impact does not'); + }); + + it('renders failed enrichment as unavailable and fails the risk gate closed', async () => { + const executeQuery = vi.fn(async (query: string) => { + if (query.includes("WHERE n.name = 'target'")) { + return [{ id: 'target-id', nodeType: 'Function', filePath: 'src/target.ts' }]; + } + if (query.includes('MATCH (affected)-[r:CodeRelation]->(target)')) { + return [ + { + id: 'caller-id', + name: 'caller', + nodeType: 'Function', + filePath: 'src/caller.ts', + startLine: 4, + edgeType: 'CALLS', + confidence: 1, + }, + ]; + } + if (query.includes('STEP_IN_PROCESS')) throw new Error('process query failed'); + if (query.includes('MEMBER_OF')) return []; + return []; + }); + + const output = await impactTool({ ...noOpBackend, executeQuery }).invoke({ + target: 'target', + direction: 'upstream', + maxDepth: 1, + }); + + expect(output).toContain('AFFECTED PROCESSES:\n- Unavailable (enrichment query failed)'); + expect(output).not.toContain('AFFECTED PROCESSES:\n- None found'); + expect(output).toContain('RISK: UNKNOWN'); + expect(output).toContain('risk is unresolved because enrichment failed'); + expect(output).toContain('- Processes affected: unavailable'); + }); + + it('preserves proved CRITICAL risk when the cluster query fails', async () => { + const executeQuery = vi.fn(async (query: string) => { + if (query.includes("WHERE n.name = 'target'")) { + return [{ id: 'target-id', nodeType: 'Function', filePath: 'src/target.ts' }]; + } + if (query.includes('MATCH (affected)-[r:CodeRelation]->(target)')) { + return [ + { + id: 'caller-id', + name: 'caller', + nodeType: 'Function', + filePath: 'src/caller.ts', + edgeType: 'CALLS', + confidence: 1, + }, + ]; + } + if (query.includes('STEP_IN_PROCESS')) { + return Array.from({ length: 5 }, (_, index) => ({ + label: `process-${index}`, + hits: 1, + minStep: index + 1, + stepCount: 5, + })); + } + if (query.includes('MEMBER_OF') && query.includes('COUNT(DISTINCT s.id)')) { + throw new Error('cluster query failed'); + } + if (query.includes('MEMBER_OF')) return []; + return []; + }); + + const output = await impactTool({ ...noOpBackend, executeQuery }).invoke({ + target: 'target', + direction: 'upstream', + maxDepth: 1, + }); + + expect(output).toContain('RISK: CRITICAL'); + expect(output).toContain('AFFECTED CLUSTERS:\n- Unavailable (enrichment query failed)'); + expect(output).toContain('- Processes affected: 5'); + expect(output).toContain('- Clusters affected: unavailable'); + }); + + it('does not invent direct/indirect cluster classification after its query fails', async () => { + const executeQuery = vi.fn(async (query: string) => { + if (query.includes("WHERE n.name = 'target'")) { + return [{ id: 'target-id', nodeType: 'Function', filePath: 'src/target.ts' }]; + } + if (query.includes('MATCH (affected)-[r:CodeRelation]->(target)')) { + return [ + { + id: 'caller-id', + name: 'caller', + nodeType: 'Function', + filePath: 'src/caller.ts', + edgeType: 'CALLS', + confidence: 1, + }, + ]; + } + if (query.includes('STEP_IN_PROCESS')) return []; + if (query.includes('MEMBER_OF') && query.includes('RETURN DISTINCT')) { + throw new Error('classification query failed'); + } + if (query.includes('MEMBER_OF')) return [{ label: 'Core', hits: 1 }]; + return []; + }); + + const output = await impactTool({ ...noOpBackend, executeQuery }).invoke({ + target: 'target', + direction: 'upstream', + maxDepth: 1, + }); + + expect(output).toContain('- Core (classification-unavailable, 1 symbols)'); + expect(output).toContain('direct/indirect cluster classification is unavailable'); + expect(output).not.toContain('process/module axes were unused'); + }); + + it('treats successful File expansion as comparable because enrichment runs on member symbols', async () => { + const executeQuery = vi.fn(async (query: string) => { + if (query.includes("n.filePath CONTAINS 'src/target.ts'")) { + return [{ id: 'file-id', nodeType: 'File', filePath: 'src/target.ts' }]; + } + if (query.includes("callee.filePath = 'src/target.ts'")) { + return [ + { + id: 'caller-id', + name: 'caller', + nodeType: 'Function', + filePath: 'src/caller.ts', + edgeType: 'CALLS', + confidence: 1, + }, + ]; + } + if (query.includes('STEP_IN_PROCESS')) { + return [{ label: 'Build', hits: 1, minStep: 1, stepCount: 1 }]; + } + if (query.includes('MEMBER_OF') && query.includes('RETURN DISTINCT')) { + return [{ label: 'Core' }]; + } + if (query.includes('MEMBER_OF')) return [{ label: 'Core', hits: 1 }]; + return []; + }); + + const output = await impactTool({ ...noOpBackend, executeQuery }).invoke({ + target: 'src/target.ts', + direction: 'upstream', + maxDepth: 1, + }); + + expect(output).toContain('process/cluster axes are comparable here when enrichment succeeds'); + expect(output).toContain('- Processes affected: 1'); + expect(output).toContain('- Clusters affected: 1'); + expect(output).not.toContain('process/module axes were unused'); + }); + + it('surfaces the 500-symbol enrichment cap as partial', async () => { + const executeQuery = vi.fn(async (query: string) => { + if (query.includes("WHERE n.name = 'target'")) { + return [{ id: 'target-id', nodeType: 'Function', filePath: 'src/target.ts' }]; + } + const depth = query.includes('3 AS depth') ? 3 : query.includes('2 AS depth') ? 2 : 1; + if (query.includes('CodeRelation') && query.includes(` ${depth} AS depth`)) { + return Array.from({ length: 200 }, (_, index) => ({ + id: `d${depth}-${index}`, + name: `node-${depth}-${index}`, + nodeType: 'Function', + filePath: `src/d${depth}-${index}.ts`, + edgeType: 'CALLS', + confidence: 1, + })); + } + if (query.includes('STEP_IN_PROCESS') || query.includes('MEMBER_OF')) return []; + return []; + }); + + const output = await impactTool({ ...noOpBackend, executeQuery }).invoke({ + target: 'target', + direction: 'upstream', + maxDepth: 3, + }); + + expect(output).toContain('process/cluster enrichment is partial (first 500 symbols)'); + expect(output).toContain('enrichment-truncated'); + expect(output).not.toContain('enrichment-budget-exhausted'); + }); +}); diff --git a/gitnexus/skills/gitnexus-impact-analysis.md b/gitnexus/skills/gitnexus-impact-analysis.md index 4fb73f3e6..85d90c90d 100644 --- a/gitnexus/skills/gitnexus-impact-analysis.md +++ b/gitnexus/skills/gitnexus-impact-analysis.md @@ -93,6 +93,15 @@ dispatch, cross-language calls), so few-callers ⇒ LOW does **not** apply. The result carries a `riskNote` saying so. Confirm with a text search before treating the symbol as safe to change or delete. +`risk` is the edit gate: warn on HIGH/CRITICAL and stop on UNKNOWN until the +uncertainty is resolved. Within single-repo mode, compare File and symbol +targets with local `riskSharedAxes` (direct/total only). Within group mode, +compare only group results: their `riskSharedAxes` overlays resolved +cross-repo crossings on that local value. Never use either field to waive the +edit gate. Check `riskScale.unusedAxes` before comparing kinds: MCP File walks +omit process/module axes, while web Graph-RAG expands File targets to in-file +symbols before enrichment. + ## Tools **impact** — the primary tool for symbol blast radius. If MCP is unavailable, use `node .gitnexus/run.cjs impact --direction upstream --repo .` instead: diff --git a/gitnexus/src/cli/ai-context.ts b/gitnexus/src/cli/ai-context.ts index 258aeb46c..cb7b42c60 100644 --- a/gitnexus/src/cli/ai-context.ts +++ b/gitnexus/src/cli/ai-context.ts @@ -218,16 +218,16 @@ This project is indexed by GitNexus as **${projectName}**${noStats ? '' : ` (${s ## Always Do -- **MUST run impact analysis before editing.** Use \`impact({target: "symbolName", direction: "upstream"})\` (MCP) or \`${runner} impact "symbolName" --direction upstream --repo .\` (CLI fallback); report callers, processes, and risk. Never substitute grep for graph analysis.${ +- **MUST run impact before editing.** Use \`impact({target: "symbolName", direction: "upstream"})\` or \`${runner} impact "symbolName" --direction upstream --repo .\`; report callers, processes, and risk. Never substitute grep for graph analysis.${ hasPdg ? ` For unified PDG impact, add \`mode: "pdg"\` with optional \`line: \` — it returns statement-level \`affectedStatements\` over CDG + REACHING_DEF and inter-procedural symbols in \`interproceduralByDepth\`/\`byDepth\`; no-layer/degraded PDG results are UNKNOWN-risk notes (\`--pdg\` layer). CLI equivalent: \`${runner} impact "symbolName" --direction upstream --mode pdg --line --repo .\`.` : '' } - **MUST analyze graph changes before committing.** Use \`detect_changes({scope: "all"})\` (MCP) or \`${runner} detect-changes --scope all --repo .\` (CLI fallback). \`partial: true\` or \`truncated: true\` is not a clean check — a zero means unseen, not unaffected; re-run it. For regression review: \`detect_changes({scope: "compare", base_ref: ${JSON.stringify(markdownSafeBranch(defaultBranch))}})\` or \`${runner} detect-changes --scope compare --base-ref ${JSON.stringify(markdownSafeBranch(defaultBranch))} --repo .\`. -- **MUST warn the user** if impact analysis returns HIGH or CRITICAL risk before proceeding with edits. +- MUST warn on HIGH/CRITICAL \`risk\` pre-edit; never use \`riskSharedAxes\` to waive a HIGH/CRITICAL \`risk\` warning. Compare File/symbol: MCP File omits axes; Graph-RAG expands File. - **MUST treat \`risk: UNKNOWN\` as unresolved, not as low.** An empty caller set is not evidence the symbol is unused — it can also mean the callers are not resolvable by the index (plain-object property access, dynamic dispatch, cross-language calls). \`impact\` pairs \`UNKNOWN\` with a \`riskNote\` saying so. Confirm with a text search before treating the symbol as safe to change or delete; do not proceed on the strength of a zero. -- When exploring unfamiliar code, use \`query({search_query: "concept"})\` to find execution flows instead of grepping. It returns process-grouped results ranked by relevance. -- When you need full context on a specific symbol — callers, callees, which execution flows it participates in — use \`context({name: "symbolName"})\`. +- Explore with \`query({search_query: "concept"})\` for process-grouped flows. +- Use \`context({name: "symbolName"})\` for callers, callees, and flows. - For security review, \`explain({target: "fileOrSymbol"})\` lists taint findings (source→sink flows; needs \`analyze --pdg\`).${ hasPdg ? `\n- For control/data dependence, \`pdg_query({mode: "controls", target: "fileOrSymbol"})\` answers "under what condition does X run?" (CDG, incl. guard clauses) and \`pdg_query({mode: "flows", target, variable})\` traces "where does variable Y flow?" (REACHING_DEF). \`--pdg\` layer.` diff --git a/gitnexus/src/cli/eval-server.ts b/gitnexus/src/cli/eval-server.ts index caf19bc03..b2f656c39 100644 --- a/gitnexus/src/cli/eval-server.ts +++ b/gitnexus/src/cli/eval-server.ts @@ -302,6 +302,20 @@ function formatTruncationSuffix(result: { return label ? ` (by ${label})` : ''; } +function pushCallgraphRiskLines(lines: string[], result: any): void { + if (result.risk) { + lines.push(`Risk: ${result.risk}`); + } + if (result.riskNote) { + lines.push(String(result.riskNote)); + } + if (result.riskScale?.comparableAcrossKinds === false && result.riskSharedAxes) { + lines.push( + `Shared-axes risk: ${result.riskSharedAxes} (process/module axes are unavailable — compare File vs symbol only; do not use this to waive a HIGH/CRITICAL risk warning)`, + ); + } +} + export function formatImpactResult(result: any): string { if (result.error) { const suggestion = result.suggestion ? `\nSuggestion: ${result.suggestion}` : ''; @@ -567,14 +581,21 @@ export function formatImpactResult(result: any): string { // #1858 — "isolated" is a confident claim. If an interface / indirection // boundary is on the path, the true count is a lower bound, not zero; // callers binding via DI / dynamic dispatch were not traced. Say so instead. + const lines: string[] = []; if (result.epistemic === 'lower-bound') { - const lines = [ + lines.push( `${target?.name || '?'}: no direct ${direction} dependencies traced, but this is a LOWER BOUND — unresolved indirection on the path (actual impact may be higher):`, - ]; + ); for (const b of result.boundaries || []) lines.push(` • ${b}`); - return lines.join('\n'); + } else if (direction === 'upstream') { + lines.push( + `${target?.name || '?'}: No ${direction} callers resolved. This is not evidence the symbol is unused or isolated.`, + ); + } else { + lines.push(`${target?.name || '?'}: No ${direction} dependencies found.`); } - return `${target?.name || '?'}: No ${direction} dependencies found. This symbol appears isolated.`; + pushCallgraphRiskLines(lines, result); + return lines.join('\n'); } const lines: string[] = []; @@ -594,6 +615,7 @@ export function formatImpactResult(result: any): string { ); for (const b of result.boundaries || []) lines.push(` • ${b}`); } + pushCallgraphRiskLines(lines, result); lines.push(''); const depthLabels: Record = { diff --git a/gitnexus/src/core/group/cross-impact.ts b/gitnexus/src/core/group/cross-impact.ts index 19ddd6fee..485b1ee8c 100644 --- a/gitnexus/src/core/group/cross-impact.ts +++ b/gitnexus/src/core/group/cross-impact.ts @@ -5,6 +5,7 @@ import fsp from 'node:fs/promises'; import path from 'node:path'; +import type { ImpactRisk } from 'gitnexus-shared'; import type { BridgeHandle, BridgeMeta, @@ -381,7 +382,17 @@ function extractProcessNames(impact: unknown): string[] { // permanently that a PDG `risk:'UNKNOWN'` never coalesces to a confident `LOW`. // No behavior change — `'UNKNOWN'` was already handled correctly at the // `(localRisk === 'LOW' || localRisk === 'UNKNOWN')` branch below. -export function mergeRisk(localRisk: string, cross: CrossRepoImpact[]): string { +function asImpactRisk(value: unknown, fallback: ImpactRisk = 'LOW'): ImpactRisk { + return value === 'LOW' || + value === 'MEDIUM' || + value === 'HIGH' || + value === 'CRITICAL' || + value === 'UNKNOWN' + ? value + : fallback; +} + +export function mergeRisk(localRisk: ImpactRisk, cross: CrossRepoImpact[]): ImpactRisk { const traversed = cross.filter((c) => c.fanout_status !== 'not_attempted'); const highConf = traversed.some((c) => c.contract.confidence >= 0.85); if (localRisk === 'CRITICAL') return 'CRITICAL'; @@ -391,6 +402,22 @@ export function mergeRisk(localRisk: string, cross: CrossRepoImpact[]): string { return localRisk; } +function liftLocalRiskMeta( + local: unknown, + cross: CrossRepoImpact[], +): Pick { + const { riskSharedAxes, riskScale } = local as { + riskSharedAxes?: unknown; + riskScale?: GroupImpactResult['riskScale']; + }; + return { + ...(riskSharedAxes !== undefined + ? { riskSharedAxes: mergeRisk(asImpactRisk(riskSharedAxes), cross) } + : {}), + ...(riskScale !== undefined ? { riskScale } : {}), + }; +} + /** * Is this bridge's metadata unable to say where its contents came from? * @@ -601,6 +628,7 @@ export async function runGroupImpact( cross_repo_hits: 0, }, risk: 'UNKNOWN', + ...liftLocalRiskMeta(local, []), timeoutMs, crossDepthWarning, }; @@ -656,7 +684,8 @@ export async function runGroupImpact( modules_affected: s.modules_affected ?? 0, cross_repo_hits: 0, }, - risk: String((local as { risk?: string }).risk ?? 'LOW'), + risk: asImpactRisk((local as { risk?: unknown }).risk), + ...liftLocalRiskMeta(local, []), timeoutMs, crossDepthWarning, }; @@ -826,7 +855,7 @@ export async function runGroupImpact( } const localSum = (local as { summary?: Record })?.summary || {}; - const localRisk = String((local as { risk?: string }).risk ?? 'LOW'); + const localRisk = asImpactRisk((local as { risk?: unknown }).risk); const localPartial = Boolean((local as { partial?: boolean }).partial); // The bridge's own incompleteness, in the shared vocabulary, read through // what this query DECLARED. The fan-out above already drops every neighbour @@ -905,6 +934,7 @@ export async function runGroupImpact( cross_repo_hits: cross.length, }, risk: mergeRisk(localRisk, cross), + ...liftLocalRiskMeta(local, cross), timeoutMs, crossDepthWarning, }; diff --git a/gitnexus/src/core/group/types.ts b/gitnexus/src/core/group/types.ts index beeb6f053..023629506 100644 --- a/gitnexus/src/core/group/types.ts +++ b/gitnexus/src/core/group/types.ts @@ -1,3 +1,5 @@ +import type { ImpactRisk, ImpactRiskResult } from 'gitnexus-shared'; + export type ContractType = | 'http' | 'graphql' @@ -194,7 +196,17 @@ export interface GroupImpactResult { modules_affected: number; cross_repo_hits: number; }; - risk: string; + risk: ImpactRisk; + /** + * Two-axis (direct + total) risk from the local leg, then `mergeRisk` with + * crossings — compare File vs symbol here, not via top-level `risk`. + */ + riskSharedAxes?: ImpactRisk; + /** + * Local-leg scale metadata (File / skipped enrichment). Crossings do not + * invent process/module membership for File nodes. + */ + riskScale?: ImpactRiskResult['riskScale']; /** * `'lower-bound'` when the fan-out was cut short, so `risk` is a FLOOR, not a * verdict. Same vocabulary as single-repo `impact`'s `epistemic` field. diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 6451b0d9b..544f64c12 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -9,6 +9,7 @@ import fs from 'fs/promises'; import path from 'path'; import { createHash } from 'crypto'; +import { scoreImpactRisk, unusedAxesForImpactWalk, type ImpactRiskResult } from 'gitnexus-shared'; import { initLbug, executeQuery, @@ -6412,6 +6413,8 @@ export class LocalBackend { let summary: { impactedCount: number; risk: string; + riskSharedAxes?: string; + riskScale?: ImpactRiskResult['riskScale']; riskNote?: string; summary?: { direct: number }; } | null = null; @@ -6451,6 +6454,10 @@ export class LocalBackend { score: Number(c.score.toFixed(2)), impactedCount: summary?.impactedCount ?? 0, risk: summary?.risk ?? 'UNKNOWN', + ...(summary?.riskSharedAxes !== undefined + ? { riskSharedAxes: summary.riskSharedAxes } + : {}), + ...(summary?.riskScale !== undefined ? { riskScale: summary.riskScale } : {}), direct: summary?.summary?.direct ?? 0, ...(summary?.riskNote !== undefined ? { riskNote: summary.riskNote } : {}), // Carry the explanation with the verdict. The single-symbol path @@ -7504,6 +7511,10 @@ export class LocalBackend { const parsedMaxChunks = rawMaxChunks ? Number(rawMaxChunks) : Number.NaN; const MAX_CHUNKS = Number.isInteger(parsedMaxChunks) && parsedMaxChunks >= 0 ? parsedMaxChunks : 10; + let processQueryFailed = false; + let moduleQueryFailed = false; + let enrichmentDegraded = false; + let moduleClassificationFailed = false; // `skipEnrichment` (ambiguous #2129 per-candidate probes) bypasses the // process/module aggregation passes entirely — those probes need only the @@ -7554,7 +7565,12 @@ export class LocalBackend { ORDER BY pId `, { ids }, - ).catch(() => []); + ).catch((err) => { + processQueryFailed = true; + enrichmentDegraded = true; + logQueryError('impact:process-chunk', err); + return []; + }); for (const row of rows) { const pId = row.pId ?? row[0]; @@ -7605,6 +7621,8 @@ export class LocalBackend { ep.earliest_broken_step = Math.min(ep.earliest_broken_step, minStep ?? Infinity); } } catch (e) { + processQueryFailed = true; + enrichmentDegraded = true; logQueryError('impact:process-chunk', e); } } @@ -7624,7 +7642,11 @@ export class LocalBackend { RETURN p.id AS pid, MIN(r.step) AS minStep `, { pIds, ids: allImpactedIds }, - ).catch(() => []); + ).catch((err) => { + enrichmentDegraded = true; + logQueryError('impact:process-chunk-backfill', err); + return []; + }); for (const mr of missingRows) { const pid = mr.pid ?? mr[0]; @@ -7638,6 +7660,7 @@ export class LocalBackend { } } } catch (e) { + enrichmentDegraded = true; logQueryError('impact:process-chunk-backfill', e); } } @@ -7698,7 +7721,12 @@ export class LocalBackend { LIMIT 20 `, { ids: idsChunk }, - ).catch(() => []); + ).catch((err) => { + moduleQueryFailed = true; + enrichmentDegraded = true; + logQueryError('impact:module-chunk', err); + return []; + }); for (const r of rows) { const name = r.name ?? r[0] ?? null; @@ -7707,6 +7735,8 @@ export class LocalBackend { moduleHitsMap.set(name, (moduleHitsMap.get(name) || 0) + hits); } } catch (e) { + moduleQueryFailed = true; + enrichmentDegraded = true; logQueryError('impact:module-chunk', e); } }; @@ -7732,12 +7762,19 @@ export class LocalBackend { RETURN DISTINCT c.heuristicLabel AS name `, { ids: idsChunk }, - ).catch(() => []); + ).catch((err) => { + enrichmentDegraded = true; + moduleClassificationFailed = true; + logQueryError('impact:direct-module-chunk', err); + return []; + }); for (const r of rows) { const name = r.name ?? r[0] ?? null; if (name) directModuleSet.add(name); } } catch (e) { + enrichmentDegraded = true; + moduleClassificationFailed = true; logQueryError('impact:direct-module-chunk', e); } }; @@ -7762,7 +7799,11 @@ export class LocalBackend { return { name, hits, - impact: directModuleNameSet.has(name) ? 'direct' : 'indirect', + impact: moduleClassificationFailed + ? 'classification-unavailable' + : directModuleNameSet.has(name) + ? 'direct' + : 'indirect', }; }); } @@ -7770,40 +7811,25 @@ export class LocalBackend { // Risk scoring const processCount = affectedProcesses.length; const moduleCount = affectedModules.length; - 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 || - processCount >= 3 || - moduleCount >= 3 || - impacted.length >= 100 - ) { - risk = 'HIGH'; - } else if (directCount >= 5 || impacted.length >= 30) { - risk = 'MEDIUM'; - } else { - risk = 'LOW'; - } + const isFileTarget = symType === 'File' || String(symId).startsWith('File:'); + const unusedAxes = unusedAxesForImpactWalk({ + isFileTarget, + skipEnrichment, + maxChunks: MAX_CHUNKS, + processQueryFailed, + moduleQueryFailed, + impactedCount: impacted.length, + enrichmentTruncated: + !skipEnrichment && MAX_CHUNKS > 0 && impacted.length > MAX_CHUNKS * CHUNK_SIZE, + }); + const { risk, riskSharedAxes, riskScale } = scoreImpactRisk({ + direction, + directCount, + processCount, + moduleCount, + impactedCount: impacted.length, + unusedAxes, + }); // Build per-depth counts (always included, even in summaryOnly mode) const byDepthCounts: Record = {}; @@ -7823,7 +7849,7 @@ export class LocalBackend { target: { id: symId, name: sym.name || sym[1], - type: symType, + type: isFileTarget ? symType || 'File' : symType, filePath: sym.filePath || sym[2], ...(beanMetadata ? { bean: beanMetadata } : {}), ...(aopMetadata ? { aop: aopMetadata } : {}), @@ -7831,18 +7857,27 @@ export class LocalBackend { direction, impactedCount: impacted.length, risk, + riskSharedAxes, + riskScale, ...(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.', + processQueryFailed || moduleQueryFailed + ? 'Risk is unresolved because process/module enrichment failed. Observed counts ' + + 'are lower bounds; retry impact before treating the change as safe.' + : unusedAxes.some((axis) => axis.reason === 'enrichment-truncated') + ? 'Risk is unresolved because process/module enrichment was truncated. Observed ' + + 'counts are lower bounds; retry with a higher IMPACT_MAX_CHUNKS before ' + + 'treating the change as safe.' + : '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 }), + ...((!traversalComplete || enrichmentDegraded) && { partial: true }), summary: { direct: directCount, processes_affected: processCount, diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index 53700a7a2..4244a95bf 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -475,11 +475,13 @@ 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. 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. +- risk: LOW / MEDIUM / HIGH / CRITICAL / UNKNOWN. This is the HIGH/CRITICAL edit-gate field. File targets lack process/community membership, so their risk is not directly comparable with symbol risk; use riskSharedAxes to compare the direct/total axes common to both. Group-mode (\`repo: "@…"\`) results lift the same fields to the top-level envelope. The web Graph-RAG impact tool expands File targets to in-file symbols before enrichment, so process/cluster axes remain comparable there. 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. +- riskSharedAxes: single-repo risk computed only from direct and total impact. Group mode then applies the cross-repo crossing overlay to that local value. Suitable for comparing File and symbol targets within the same mode. Never substitute it for \`risk\` when deciding whether to warn before edits. +- riskScale: { comparableAcrossKinds, unusedAxes } — names process/module axes that were structurally unavailable, skipped, budget-exhausted (\`IMPACT_MAX_CHUNKS=0\`), truncated (sampled a subset of impacted symbols), or failed at query time. Failed-query and truncated-sample counts are lower bounds: known HIGH/CRITICAL warnings survive, otherwise risk is UNKNOWN. Group impact copies this metadata from the local leg. - 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) +- affected_modules: which functional areas are hit (direct vs indirect; classification-unavailable when that secondary query fails) - byDepth: affected symbols grouped by traversal depth (paginated by limit/offset; omitted when summaryOnly:true — use byDepthCounts for totals per depth, pagination object when truncated). Each item includes a processes:[{id,label,processType,step}] field listing the execution flows that symbol participates in. Empty when the symbol has no process membership. Can ALSO be empty when partial:true is set — either the process-aggregation pass hit its cap before detecting affected processes, or per-symbol enrichment was capped on a very large page. When partial:true, do NOT treat processes:[] as proof of no participation; cross-check the top-level affected_processes list. - epistemic: 'exact' | 'lower-bound' — whether impactedCount is the whole story. 'lower-bound' means the walk provably missed callers, so the count is a floor. Absent only on skipped probes (ambiguous-candidate lists, group fan-out). - boundaries: string[] — one plain-language sentence per reason the count is short. Prose for humans; branch on causes instead. diff --git a/gitnexus/test/integration/impact-file-risk-scale.test.ts b/gitnexus/test/integration/impact-file-risk-scale.test.ts new file mode 100644 index 000000000..16b2e6a7f --- /dev/null +++ b/gitnexus/test/integration/impact-file-risk-scale.test.ts @@ -0,0 +1,143 @@ +import { beforeAll, expect, it, vi } from 'vitest'; +import { LocalBackend } from '../../src/mcp/local/local-backend.js'; +import { listRegisteredRepos } from '../../src/storage/repo-manager.js'; +import { withTestLbugDB, type IndexedDBHandle } 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 fileImporters = Array.from( + { length: 13 }, + (_, index) => + `CREATE (f:File {id: 'File:src/importer-${index}.ts', name: 'importer-${index}.ts', filePath: 'src/importer-${index}.ts', content: ''})`, +); +const fileImportEdges = Array.from( + { length: 13 }, + (_, index) => + `MATCH (a:File {id:'File:src/importer-${index}.ts'}), (b:File {id:'File:src/crypto.ts'}) CREATE (a)-[:CodeRelation {type:'IMPORTS', confidence:1.0, reason:'import', step:0}]->(b)`, +); +const processNodes = Array.from( + { length: 4 }, + (_, index) => + `CREATE (p:Process {id: 'proc-${index}', label: 'Flow ${index}', heuristicLabel: 'Flow ${index}', processType: 'cross_community', stepCount: 3, communities: [], entryPointId: 'Function:src/entry-${index}.ts:entry${index}', terminalId: 'Function:src/crypto.ts:getEncryptionKey'})`, +); +const processEntryPoints = Array.from( + { length: 4 }, + (_, index) => + `CREATE (ep:Function {id: 'Function:src/entry-${index}.ts:entry${index}', name: 'entry${index}', filePath: 'src/entry-${index}.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, +); +const processEdges = Array.from( + { length: 4 }, + (_, index) => + `MATCH (a:Function {id:'Function:src/caller-${index % 2}.ts:caller${index % 2}'}), (p:Process {id:'proc-${index}'}) CREATE (a)-[:CodeRelation {type:'STEP_IN_PROCESS', confidence:1.0, reason:'trace-detection', step:1}]->(p)`, +); + +const SEED = [ + `CREATE (f:File {id: 'File:src/crypto.ts', name: 'crypto.ts', filePath: 'src/crypto.ts', content: ''})`, + ...fileImporters, + ...fileImportEdges, + `CREATE (fn:Function {id: 'Function:src/crypto.ts:getEncryptionKey', name: 'getEncryptionKey', filePath: 'src/crypto.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, + `CREATE (c0:Function {id: 'Function:src/caller-0.ts:caller0', name: 'caller0', filePath: 'src/caller-0.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, + `CREATE (c1:Function {id: 'Function:src/caller-1.ts:caller1', name: 'caller1', filePath: 'src/caller-1.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, + `MATCH (a:Function {id:'Function:src/caller-0.ts:caller0'}), (b:Function {id:'Function:src/crypto.ts:getEncryptionKey'}) CREATE (a)-[:CodeRelation {type:'CALLS', confidence:1.0, reason:'direct', step:0}]->(b)`, + `MATCH (a:Function {id:'Function:src/caller-1.ts:caller1'}), (b:Function {id:'Function:src/crypto.ts:getEncryptionKey'}) CREATE (a)-[:CodeRelation {type:'CALLS', confidence:1.0, reason:'direct', step:0}]->(b)`, + ...processEntryPoints, + ...processNodes, + ...processEdges, +]; + +type BackendHandle = IndexedDBHandle & { _backend?: LocalBackend }; + +withTestLbugDB( + 'impact-file-risk-scale', + (handle) => { + let backend: LocalBackend; + beforeAll(() => { + const ext = handle as BackendHandle; + if (!ext._backend) throw new Error('LocalBackend not initialized'); + backend = ext._backend; + }); + + it('marks the wider File score incomparable with the process-rich Function score', async () => { + const file = await backend.callTool('impact', { + target: 'crypto.ts', + kind: 'File', + direction: 'upstream', + }); + const fn = await backend.callTool('impact', { + target: 'getEncryptionKey', + kind: 'Function', + direction: 'upstream', + }); + + expect(file.impactedCount).toBe(13); + expect(file.risk).toBe('MEDIUM'); + expect(file.riskSharedAxes).toBe('MEDIUM'); + expect(file.target.type).toBe('File'); + expect(file.riskScale).toEqual({ + comparableAcrossKinds: false, + unusedAxes: [ + { + axis: 'processes', + reason: 'file-nodes-have-no-process-or-community-membership', + }, + { + axis: 'modules', + reason: 'file-nodes-have-no-process-or-community-membership', + }, + ], + }); + expect(file.riskNote).toBeUndefined(); + + expect(fn.impactedCount).toBe(2); + expect(fn.risk).toBe('HIGH'); + expect(fn.riskSharedAxes).toBe('LOW'); + expect(fn.summary.processes_affected).toBe(4); + expect(fn.riskScale).toEqual({ + comparableAcrossKinds: true, + unusedAxes: [], + }); + expect(fn.riskNote).toBeUndefined(); + }); + + it('marks downstream File risk incomparable on the same seed', async () => { + const file = await backend.callTool('impact', { + target: 'crypto.ts', + kind: 'File', + direction: 'downstream', + }); + expect(file.target.type).toBe('File'); + expect(file.riskScale.comparableAcrossKinds).toBe(false); + expect(file.riskScale.unusedAxes).toEqual( + expect.arrayContaining([ + { + axis: 'processes', + reason: 'file-nodes-have-no-process-or-community-membership', + }, + ]), + ); + }); + }, + { + 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: 14, nodes: 21, communities: 0, processes: 4 }, + }, + ]); + const backend = new LocalBackend(); + await backend.init(); + (handle as BackendHandle)._backend = backend; + }, + }, +); diff --git a/gitnexus/test/integration/impact-zero-caller-risk.test.ts b/gitnexus/test/integration/impact-zero-caller-risk.test.ts index 3438ffc46..ad6653916 100644 --- a/gitnexus/test/integration/impact-zero-caller-risk.test.ts +++ b/gitnexus/test/integration/impact-zero-caller-risk.test.ts @@ -71,6 +71,8 @@ withTestLbugDB( expect(result).not.toHaveProperty('error'); expect(result.impactedCount).toBe(0); expect(result.risk).toBe('UNKNOWN'); + expect(result.riskScale.comparableAcrossKinds).toBe(true); + expect(result.riskNote).toBeDefined(); }); // The ambiguous fan-out narrows candidates into a fresh object, and that @@ -93,6 +95,11 @@ withTestLbugDB( expect(c.risk).toBe('UNKNOWN'); expect(typeof c.riskNote).toBe('string'); expect(c.riskNote).toMatch(/not evidence/i); + expect( + (c as { riskScale?: { unusedAxes?: { reason: string }[] } }).riskScale?.unusedAxes, + ).toEqual( + expect.arrayContaining([expect.objectContaining({ reason: 'enrichment-skipped' })]), + ); } }); diff --git a/gitnexus/test/unit/ai-context-unknown-risk-policy.test.ts b/gitnexus/test/unit/ai-context-unknown-risk-policy.test.ts index b906a1975..37f13de8b 100644 --- a/gitnexus/test/unit/ai-context-unknown-risk-policy.test.ts +++ b/gitnexus/test/unit/ai-context-unknown-risk-policy.test.ts @@ -30,6 +30,9 @@ describe('generateGitNexusContent keeps the risk: UNKNOWN policy unconditional ( const content = generateGitNexusContent('UnknownRiskProject', stats, { hasPdg }); expect(content).toContain('MUST treat `risk: UNKNOWN` as unresolved, not as low.'); + expect(content).toContain( + 'never use `riskSharedAxes` to waive a HIGH/CRITICAL `risk` warning', + ); expect(content).toContain( 'callers are not resolvable by the index (plain-object property access, dynamic dispatch, cross-language calls)', ); diff --git a/gitnexus/test/unit/ai-context.test.ts b/gitnexus/test/unit/ai-context.test.ts index d8cf4b208..1e2611df8 100644 --- a/gitnexus/test/unit/ai-context.test.ts +++ b/gitnexus/test/unit/ai-context.test.ts @@ -225,6 +225,16 @@ describe('generateAIContextFiles', () => { expect(withoutPdg).toContain('explain('); }); + it('documents the MCP and Graph-RAG File-risk scale difference', () => { + const content = generateGitNexusContent('RiskScaleProject', { + nodes: 50, + edges: 100, + processes: 5, + }); + expect(content).toContain('MCP File omits axes'); + expect(content).toContain('Graph-RAG expands File'); + }); + it('emits MD060-compatible compact tables in generated docs (#2709)', () => { const content = generateGitNexusContent('MarkdownProject', { nodes: 50, diff --git a/gitnexus/test/unit/cli-impact-pdg-format.test.ts b/gitnexus/test/unit/cli-impact-pdg-format.test.ts index 9e9f4870f..e04f524af 100644 --- a/gitnexus/test/unit/cli-impact-pdg-format.test.ts +++ b/gitnexus/test/unit/cli-impact-pdg-format.test.ts @@ -498,9 +498,10 @@ describe('formatImpactResult — callgraph rendering is UNCHANGED (regression gu }, }; - it('renders the callgraph result with the exact pre-U5 text (byte-identical)', () => { + it('renders the callgraph result with Risk on the callgraph contract (byte-identical for U5+risk)', () => { const expected = [ 'Blast radius for Function computeTotal (upstream): 2 symbol(s) depends on this (will break if changed)', + 'Risk: MEDIUM', '', 'd=1: WILL BREAK (direct) (1)', ' Function callerA → src/a.ts [CALLS]', @@ -532,14 +533,14 @@ describe('formatImpactResult — callgraph rendering is UNCHANGED (regression gu expect(out).not.toContain('PDG-dependent symbols'); }); - it('renders the callgraph isolated / zero case unchanged', () => { + it('renders the callgraph isolated / zero case with risk, without claiming isolation', () => { const out = formatImpactResult({ target: { name: 'lonely' }, direction: 'downstream', impactedCount: 0, risk: 'LOW', }); - expect(out).toBe('lonely: No downstream dependencies found. This symbol appears isolated.'); + expect(out).toBe('lonely: No downstream dependencies found.\nRisk: LOW'); }); it('renders the callgraph lower-bound (DI/dynamic-dispatch) copy unchanged', () => { diff --git a/gitnexus/test/unit/eval-formatters.test.ts b/gitnexus/test/unit/eval-formatters.test.ts index c0349e80d..cdd12b4fd 100644 --- a/gitnexus/test/unit/eval-formatters.test.ts +++ b/gitnexus/test/unit/eval-formatters.test.ts @@ -409,6 +409,35 @@ describe('formatImpactResult', () => { expect(result).toContain('caller2'); }); + it('prints the shared-axes comparison when a target has unavailable risk axes', () => { + const result = formatImpactResult({ + target: { kind: 'File', name: 'crypto.ts' }, + direction: 'upstream', + impactedCount: 13, + risk: 'MEDIUM', + riskSharedAxes: 'MEDIUM', + riskScale: { + comparableAcrossKinds: false, + unusedAxes: [ + { + axis: 'processes', + reason: 'file-nodes-have-no-process-or-community-membership', + }, + { + axis: 'modules', + reason: 'file-nodes-have-no-process-or-community-membership', + }, + ], + }, + byDepthCounts: { 1: 13 }, + }); + + expect(result).toContain('Risk: MEDIUM'); + expect(result).toContain('Shared-axes risk: MEDIUM'); + expect(result).toContain('process/module axes are unavailable'); + expect(result).toContain('do not use this to waive a HIGH/CRITICAL risk warning'); + }); + it('handles zero impact', () => { const result = formatImpactResult({ target: { name: 'foo' }, @@ -416,7 +445,22 @@ describe('formatImpactResult', () => { impactedCount: 0, byDepth: {}, }); - expect(result).toContain('No upstream dependencies'); + expect(result).toContain('No upstream callers resolved'); + expect(result).not.toContain('appears isolated'); + }); + + it('prints UNKNOWN and riskNote for an empty upstream walk', () => { + const result = formatImpactResult({ + target: { name: 'foo' }, + direction: 'upstream', + impactedCount: 0, + risk: 'UNKNOWN', + riskNote: 'safe to change is a claim about callers and there were none to reason about', + byDepth: {}, + }); + expect(result).toContain('Risk: UNKNOWN'); + expect(result).toContain('safe to change is a claim about callers'); + expect(result).not.toContain('appears isolated'); }); it('formats impact by depth', () => { diff --git a/gitnexus/test/unit/group/cross-impact-fanout-cap.test.ts b/gitnexus/test/unit/group/cross-impact-fanout-cap.test.ts index aac1ba794..58b11451c 100644 --- a/gitnexus/test/unit/group/cross-impact-fanout-cap.test.ts +++ b/gitnexus/test/unit/group/cross-impact-fanout-cap.test.ts @@ -175,6 +175,32 @@ describe('group impact fan-out is bounded by a count, not by the clock (#2787)', expect(result).not.toHaveProperty('riskEpistemic'); }); + it('applies crossing risk to shared axes while preserving local scale metadata', async () => { + bridgeRows.value = [crossingRow(0, 0.9)]; + const riskScale = { + comparableAcrossKinds: true, + unusedAxes: [], + } as const; + const port = makePort({ + impact: vi.fn(async () => ({ + target: { id: 'Function:src/api.ts:publish', filePath: 'src/api.ts' }, + byDepth: { 1: [{ id: 'u1', filePath: 'src/a.ts' }] }, + summary: { direct: 1, processes_affected: 4, modules_affected: 0 }, + risk: 'HIGH', + riskSharedAxes: 'LOW', + riskScale, + })) as GroupToolPort['impact'], + }); + + const result = await run(port); + + expect(result).toMatchObject({ + risk: 'HIGH', + riskSharedAxes: 'HIGH', + riskScale, + }); + }); + it('marks risk as a floor when a crossing is dropped, and does NOT clamp the value down', async () => { // Three crossings, one neighbour unresolvable. Two traverse → HIGH (the // >=0.85-confidence gate). Had the third traversed, `traversed.length >= 3` diff --git a/gitnexus/test/unit/group/cross-impact.test.ts b/gitnexus/test/unit/group/cross-impact.test.ts index 9aac679a0..bceaa9195 100644 --- a/gitnexus/test/unit/group/cross-impact.test.ts +++ b/gitnexus/test/unit/group/cross-impact.test.ts @@ -371,6 +371,35 @@ describe('cross-impact', () => { expect(r).not.toHaveProperty('truncationReason'); }); + it('lifts local riskSharedAxes and riskScale when there are no symbol uids to fan out', async () => { + const riskScale = { + comparableAcrossKinds: false, + unusedAxes: [ + { + axis: 'processes' as const, + reason: 'file-nodes-have-no-process-or-community-membership', + }, + { + axis: 'modules', + reason: 'file-nodes-have-no-process-or-community-membership', + }, + ], + }; + const r = await runLocalOnlyImpact(async () => ({ + byDepth: {}, + summary: { direct: 13, processes_affected: 0, modules_affected: 0 }, + risk: 'MEDIUM', + riskSharedAxes: 'MEDIUM', + riskScale, + })); + expect(r).toMatchObject({ + truncated: false, + risk: 'MEDIUM', + riskSharedAxes: 'MEDIUM', + riskScale, + }); + }); + it('test_runGroupImpact_bridge_schema_mismatch_returns_error', async () => { const { tmpDir, groupDir, cleanup } = tmpGroup(); vi.stubEnv('GITNEXUS_HOME', tmpDir); diff --git a/gitnexus/test/unit/group/types.test.ts b/gitnexus/test/unit/group/types.test.ts index 94b0b556c..50eddf921 100644 --- a/gitnexus/test/unit/group/types.test.ts +++ b/gitnexus/test/unit/group/types.test.ts @@ -6,6 +6,7 @@ import type { CrossLink, ContractRegistry, GroupManifestLink, + GroupImpactResult, MatchType, } from '../../../src/core/group/types.js'; @@ -132,4 +133,20 @@ describe('Group types', () => { }; expect(l.contract).toBe('/x'); }); + + it('uses the shared closed union for unused impact-axis reasons', () => { + type UnusedAxis = NonNullable['unusedAxes'][number]; + const valid = { + axis: 'processes', + reason: 'enrichment-query-failed', + } satisfies UnusedAxis; + const invalid = { + axis: 'processes', + // @ts-expect-error unknown reasons must not widen the shared contract + reason: 'not-a-real-reason', + } satisfies UnusedAxis; + + expect(valid.reason).toBe('enrichment-query-failed'); + expect(invalid.reason).toBe('not-a-real-reason'); + }); }); diff --git a/gitnexus/test/unit/impact-batching-grouping.test.ts b/gitnexus/test/unit/impact-batching-grouping.test.ts index 8224cd1d3..e068870f9 100644 --- a/gitnexus/test/unit/impact-batching-grouping.test.ts +++ b/gitnexus/test/unit/impact-batching-grouping.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; // Mock the lbug-adapter module before importing LocalBackend so the class // uses the mocked implementations of executeQuery / executeParameterized. @@ -38,6 +38,10 @@ describe('impact: batching and grouping', () => { vi.clearAllMocks(); }); + afterEach(() => { + delete process.env.IMPACT_MAX_CHUNKS; + }); + it('batches 250 IDs into 3 chunked STEP_IN_PROCESS queries', async () => { // Prepare backend and a fake repo handle const backend = new LocalBackend(); @@ -320,11 +324,328 @@ describe('impact: batching and grouping', () => { expect(Array.isArray(res.affected_modules)).toBe(true); const modNames = res.affected_modules.map((m: any) => m.name); expect(modNames).toContain('ModuleA'); + expect(res.riskScale.comparableAcrossKinds).toBe(false); + expect(res.riskScale.unusedAxes).toEqual( + expect.arrayContaining([expect.objectContaining({ reason: 'enrichment-truncated' })]), + ); // Cleanup env delete process.env.IMPACT_MAX_CHUNKS; }); + it('marks IMPACT_MAX_CHUNKS=0 as unused process/module axes', async () => { + process.env.IMPACT_MAX_CHUNKS = '0'; + const backend = new LocalBackend(); + const repoHandle = { + id: 'repo-zero-budget', + name: 'repo-zero-budget', + repoPath: '/tmp/repo-zero-budget', + storagePath: '/tmp/repo-zero-budget/.gitnexus', + lbugPath: '/tmp/repo-zero-budget/.gitnexus/lbug', + indexedAt: 'now', + lastCommit: 'c', + stats: {}, + } as any; + (backend as any).repos.set(repoHandle.id, repoHandle); + (backend as any).ensureInitialized = vi.fn().mockResolvedValue(undefined); + executeQueryMock.mockImplementation(async () => []); + executeParameterizedMock.mockImplementation(async (...args: any[]) => { + const query = typeof args[1] === 'string' ? args[1] : String(args[0] ?? ''); + if (query.includes('r.type IN') && !query.includes('STEP_IN_PROCESS')) { + return [ + { + id: 'node-1', + name: 'n1', + filePath: 'file-1.js', + relType: 'CALLS', + confidence: null, + }, + ]; + } + return [{ id: 'symX', name: 'TargetX', filePath: 'f' }]; + }); + + const res = await (backend as any)._impactImpl(repoHandle, { + target: 'TargetX', + direction: 'downstream', + maxDepth: 1, + } as any); + expect(res.riskScale.comparableAcrossKinds).toBe(false); + expect(res.riskScale.unusedAxes).toEqual( + expect.arrayContaining([expect.objectContaining({ reason: 'enrichment-budget-exhausted' })]), + ); + delete process.env.IMPACT_MAX_CHUNKS; + }); + + it('marks swallowed process enrichment failures as unused process axes', async () => { + const backend = new LocalBackend(); + const repoHandle = { + id: 'repo-enrich-fail', + name: 'repo-enrich-fail', + repoPath: '/tmp/repo-enrich-fail', + storagePath: '/tmp/repo-enrich-fail/.gitnexus', + lbugPath: '/tmp/repo-enrich-fail/.gitnexus/lbug', + indexedAt: 'now', + lastCommit: 'c', + stats: {}, + } as any; + (backend as any).repos.set(repoHandle.id, repoHandle); + (backend as any).ensureInitialized = vi.fn().mockResolvedValue(undefined); + executeQueryMock.mockImplementation(async () => []); + executeParameterizedMock.mockImplementation(async (...args: any[]) => { + const query = typeof args[1] === 'string' ? args[1] : String(args[0] ?? ''); + if (query.includes('STEP_IN_PROCESS')) { + throw new Error('process chunk failed'); + } + if (query.includes('r.type IN') && !query.includes('STEP_IN_PROCESS')) { + return [ + { + id: 'node-1', + name: 'n1', + filePath: 'file-1.js', + relType: 'CALLS', + confidence: null, + }, + ]; + } + return [{ id: 'symFail', name: 'TargetFail', filePath: 'f' }]; + }); + + const res = await (backend as any)._impactImpl(repoHandle, { + target: 'TargetFail', + direction: 'downstream', + maxDepth: 1, + } as any); + expect(res.riskScale.unusedAxes).toEqual( + expect.arrayContaining([ + expect.objectContaining({ axis: 'processes', reason: 'enrichment-query-failed' }), + ]), + ); + expect(res.risk).toBe('UNKNOWN'); + expect(res.partial).toBe(true); + expect(res.riskNote).toContain('enrichment failed'); + }); + + it('marks module query failure without discarding a successful process axis', async () => { + const backend = new LocalBackend(); + const repoHandle = { + id: 'repo-module-fail', + name: 'repo-module-fail', + repoPath: '/tmp/repo-module-fail', + storagePath: '/tmp/repo-module-fail/.gitnexus', + lbugPath: '/tmp/repo-module-fail/.gitnexus/lbug', + indexedAt: 'now', + lastCommit: 'c', + stats: {}, + } as any; + (backend as any).repos.set(repoHandle.id, repoHandle); + (backend as any).ensureInitialized = vi.fn().mockResolvedValue(undefined); + executeQueryMock.mockImplementation(async () => []); + executeParameterizedMock.mockImplementation(async (...args: any[]) => { + const query = typeof args[1] === 'string' ? args[1] : String(args[0] ?? ''); + if (query.includes('MEMBER_OF')) throw new Error('module chunk failed'); + if (query.includes('STEP_IN_PROCESS') && query.includes('COUNT(DISTINCT s.id)')) { + return [ + { + pId: 'p1', + entryPointId: 'ep1', + epName: 'main', + epType: 'Function', + hits: 1, + minStep: 1, + }, + ]; + } + if (query.includes('r.type IN') && !query.includes('STEP_IN_PROCESS')) { + return [ + { + id: 'node-1', + name: 'n1', + filePath: 'file-1.js', + relType: 'CALLS', + confidence: null, + }, + ]; + } + return [{ id: 'symModule', name: 'TargetModule', filePath: 'f' }]; + }); + + const res = await (backend as any)._impactImpl(repoHandle, { + target: 'TargetModule', + direction: 'downstream', + maxDepth: 1, + } as any); + expect(res.affected_processes).toHaveLength(1); + expect(res.riskScale.unusedAxes).toEqual([ + { axis: 'modules', reason: 'enrichment-query-failed' }, + ]); + expect(res.risk).toBe('UNKNOWN'); + expect(res.partial).toBe(true); + }); + + it('keeps process risk measured when only minStep backfill fails', async () => { + const backend = new LocalBackend(); + const repoHandle = { + id: 'repo-backfill-fail', + name: 'repo-backfill-fail', + repoPath: '/tmp/repo-backfill-fail', + storagePath: '/tmp/repo-backfill-fail/.gitnexus', + lbugPath: '/tmp/repo-backfill-fail/.gitnexus/lbug', + indexedAt: 'now', + lastCommit: 'c', + stats: {}, + } as any; + (backend as any).repos.set(repoHandle.id, repoHandle); + (backend as any).ensureInitialized = vi.fn().mockResolvedValue(undefined); + executeQueryMock.mockImplementation(async () => []); + executeParameterizedMock.mockImplementation(async (...args: any[]) => { + const query = typeof args[1] === 'string' ? args[1] : String(args[0] ?? ''); + if (query.includes('MIN(r.step) AS minStep') && !query.includes('COUNT(DISTINCT s.id)')) { + throw new Error('minStep backfill failed'); + } + if (query.includes('STEP_IN_PROCESS') && query.includes('COUNT(DISTINCT s.id)')) { + return [ + { + pId: 'p1', + entryPointId: 'ep1', + epName: 'main', + epType: 'Function', + hits: 1, + minStep: null, + }, + ]; + } + if (query.includes('r.type IN') && !query.includes('STEP_IN_PROCESS')) { + return [ + { + id: 'node-1', + name: 'n1', + filePath: 'file-1.js', + relType: 'CALLS', + confidence: null, + }, + ]; + } + if (query.includes('MEMBER_OF')) return []; + return [{ id: 'symBackfill', name: 'TargetBackfill', filePath: 'f' }]; + }); + + const res = await (backend as any)._impactImpl(repoHandle, { + target: 'TargetBackfill', + direction: 'downstream', + maxDepth: 1, + } as any); + expect(res.affected_processes).toHaveLength(1); + expect(res.riskScale.unusedAxes).toEqual([]); + expect(res.risk).toBe('LOW'); + expect(res.partial).toBe(true); + }); + + it('keeps observed process warnings when a later enrichment chunk fails', async () => { + const backend = new LocalBackend(); + const repoHandle = { + id: 'repo-later-process-fail', + name: 'repo-later-process-fail', + repoPath: '/tmp/repo-later-process-fail', + storagePath: '/tmp/repo-later-process-fail/.gitnexus', + lbugPath: '/tmp/repo-later-process-fail/.gitnexus/lbug', + indexedAt: 'now', + lastCommit: 'c', + stats: {}, + } as any; + (backend as any).repos.set(repoHandle.id, repoHandle); + (backend as any).ensureInitialized = vi.fn().mockResolvedValue(undefined); + executeQueryMock.mockImplementation(async () => []); + let processChunk = 0; + executeParameterizedMock.mockImplementation(async (...args: any[]) => { + const query = typeof args[1] === 'string' ? args[1] : String(args[0] ?? ''); + if (query.includes('STEP_IN_PROCESS') && query.includes('COUNT(DISTINCT s.id)')) { + processChunk += 1; + if (processChunk === 2) throw new Error('later process chunk failed'); + return Array.from({ length: 5 }, (_, index) => ({ + pId: `p${index}`, + entryPointId: `ep${index}`, + epName: `process-${index}`, + epType: 'Function', + hits: 1, + minStep: 1, + })); + } + if (query.includes('r.type IN') && !query.includes('STEP_IN_PROCESS')) { + return Array.from({ length: 150 }, (_, index) => ({ + id: `node-${index}`, + name: `n${index}`, + filePath: `file-${index}.js`, + relType: 'CALLS', + confidence: null, + })); + } + if (query.includes('MEMBER_OF')) return []; + return [{ id: 'symLaterFail', name: 'TargetLaterFail', filePath: 'f' }]; + }); + + const res = await (backend as any)._impactImpl(repoHandle, { + target: 'TargetLaterFail', + direction: 'downstream', + maxDepth: 1, + } as any); + expect(res.affected_processes).toHaveLength(5); + expect(res.risk).toBe('CRITICAL'); + expect(res.riskScale.unusedAxes).toContainEqual({ + axis: 'processes', + reason: 'enrichment-query-failed', + }); + expect(res.partial).toBe(true); + }); + + it('does not invent direct/indirect module classification after backfill failure', async () => { + const backend = new LocalBackend(); + const repoHandle = { + id: 'repo-module-classification-fail', + name: 'repo-module-classification-fail', + repoPath: '/tmp/repo-module-classification-fail', + storagePath: '/tmp/repo-module-classification-fail/.gitnexus', + lbugPath: '/tmp/repo-module-classification-fail/.gitnexus/lbug', + indexedAt: 'now', + lastCommit: 'c', + stats: {}, + } as any; + (backend as any).repos.set(repoHandle.id, repoHandle); + (backend as any).ensureInitialized = vi.fn().mockResolvedValue(undefined); + executeQueryMock.mockImplementation(async () => []); + executeParameterizedMock.mockImplementation(async (...args: any[]) => { + const query = typeof args[1] === 'string' ? args[1] : String(args[0] ?? ''); + if (query.includes('MEMBER_OF') && query.includes('RETURN DISTINCT c.heuristicLabel')) { + throw new Error('module classification failed'); + } + if (query.includes('MEMBER_OF')) return [{ name: 'ModuleA', hits: 1 }]; + if (query.includes('STEP_IN_PROCESS')) return []; + if (query.includes('r.type IN')) { + return [ + { + id: 'node-1', + name: 'n1', + filePath: 'file-1.js', + relType: 'CALLS', + confidence: null, + }, + ]; + } + return [{ id: 'symClassify', name: 'TargetClassify', filePath: 'f' }]; + }); + + const res = await (backend as any)._impactImpl(repoHandle, { + target: 'TargetClassify', + direction: 'downstream', + maxDepth: 1, + } as any); + expect(res.affected_modules).toEqual([ + expect.objectContaining({ name: 'ModuleA', impact: 'classification-unavailable' }), + ]); + expect(res.riskScale.unusedAxes).toEqual([]); + expect(res.partial).toBe(true); + }); + it('caps implicit object-callable expansion and reports partial impact', async () => { const backend = new LocalBackend(); const repoHandle = { @@ -384,6 +705,10 @@ describe('impact: batching and grouping', () => { expect(traversalCall?.[2]?.frontierIds).toEqual(['owner']); expect(result.byDepth['1']).toHaveLength(5000); expect(result.partial).toBe(true); + expect(result.riskScale.comparableAcrossKinds).toBe(false); + expect(result.riskScale.unusedAxes).toEqual( + expect.arrayContaining([expect.objectContaining({ reason: 'enrichment-skipped' })]), + ); }); it('marks object impact partial when callable seeding fails', async () => { diff --git a/gitnexus/test/unit/impact-risk.test.ts b/gitnexus/test/unit/impact-risk.test.ts new file mode 100644 index 000000000..bd1b2661e --- /dev/null +++ b/gitnexus/test/unit/impact-risk.test.ts @@ -0,0 +1,237 @@ +import { describe, expect, it } from 'vitest'; +import { + scoreImpactRisk, + unusedAxesForImpactWalk, + type ImpactRiskInput, + type UnusedImpactRiskAxis, +} from 'gitnexus-shared'; + +const fileUnusedAxes: readonly UnusedImpactRiskAxis[] = [ + { + axis: 'processes', + reason: 'file-nodes-have-no-process-or-community-membership', + }, + { + axis: 'modules', + reason: 'file-nodes-have-no-process-or-community-membership', + }, +]; + +const base: ImpactRiskInput = { + direction: 'upstream', + directCount: 0, + processCount: 0, + moduleCount: 0, + impactedCount: 1, +}; + +describe('scoreImpactRisk', () => { + it('makes the issue #3075 File and Function scales explicit', () => { + const file = scoreImpactRisk({ + ...base, + directCount: 13, + impactedCount: 25, + unusedAxes: fileUnusedAxes, + }); + const fn = scoreImpactRisk({ + ...base, + directCount: 2, + processCount: 4, + moduleCount: 2, + impactedCount: 15, + }); + + expect(file).toEqual({ + risk: 'MEDIUM', + riskSharedAxes: 'MEDIUM', + riskScale: { + comparableAcrossKinds: false, + unusedAxes: fileUnusedAxes, + }, + }); + expect(fn).toEqual({ + risk: 'HIGH', + riskSharedAxes: 'LOW', + riskScale: { + comparableAcrossKinds: true, + unusedAxes: [], + }, + }); + }); + + it('preserves UNKNOWN only for an empty upstream walk', () => { + expect(scoreImpactRisk({ ...base, impactedCount: 0 }).risk).toBe('UNKNOWN'); + expect(scoreImpactRisk({ ...base, direction: 'downstream', impactedCount: 0 }).risk).toBe( + 'LOW', + ); + }); + + it('marks skipped enrichment as a non-comparable scale', () => { + const skippedAxes: readonly UnusedImpactRiskAxis[] = [ + { axis: 'processes', reason: 'enrichment-skipped' }, + { axis: 'modules', reason: 'enrichment-skipped' }, + ]; + + expect(scoreImpactRisk({ ...base, unusedAxes: skippedAxes }).riskScale).toEqual({ + comparableAcrossKinds: false, + unusedAxes: skippedAxes, + }); + }); + + it('preserves direct and total thresholds when enrichment axes are unused', () => { + expect( + scoreImpactRisk({ + ...base, + directCount: 30, + impactedCount: 30, + unusedAxes: fileUnusedAxes, + }).risk, + ).toBe('CRITICAL'); + expect( + scoreImpactRisk({ + ...base, + directCount: 15, + impactedCount: 15, + unusedAxes: fileUnusedAxes, + }).risk, + ).toBe('HIGH'); + expect( + scoreImpactRisk({ + ...base, + directCount: 2, + processCount: 10, + moduleCount: 10, + unusedAxes: fileUnusedAxes, + }).risk, + ).toBe('LOW'); + }); + + it('zeros unused process/module counts on the primary risk ladder', () => { + const skippedAxes: readonly UnusedImpactRiskAxis[] = [ + { axis: 'processes', reason: 'enrichment-skipped' }, + { axis: 'modules', reason: 'enrichment-skipped' }, + ]; + const scored = scoreImpactRisk({ + ...base, + directCount: 2, + processCount: 4, + moduleCount: 4, + impactedCount: 15, + unusedAxes: skippedAxes, + }); + expect(scored.risk).toBe('LOW'); + expect(scored.riskSharedAxes).toBe('LOW'); + }); + + it('preserves a warning already proved before a later enrichment failure', () => { + const scored = scoreImpactRisk({ + ...base, + directCount: 2, + processCount: 5, + impactedCount: 5, + unusedAxes: [{ axis: 'processes', reason: 'enrichment-query-failed' }], + }); + + expect(scored.risk).toBe('CRITICAL'); + expect(scored.riskSharedAxes).toBe('LOW'); + expect(scored.riskScale.comparableAcrossKinds).toBe(false); + }); + + it('fails closed when query failure leaves only a LOW or MEDIUM observed score', () => { + for (const directCount of [2, 6]) { + const scored = scoreImpactRisk({ + ...base, + direction: 'downstream', + directCount, + processCount: 0, + impactedCount: directCount, + unusedAxes: [{ axis: 'processes', reason: 'enrichment-query-failed' }], + }); + expect(scored.risk).toBe('UNKNOWN'); + } + }); + + it('fails closed when a truncated sample leaves only a LOW or MEDIUM observed score', () => { + const truncatedAxes: readonly UnusedImpactRiskAxis[] = [ + { axis: 'processes', reason: 'enrichment-truncated' }, + { axis: 'modules', reason: 'enrichment-truncated' }, + ]; + const scored = scoreImpactRisk({ + ...base, + direction: 'downstream', + directCount: 2, + processCount: 1, + moduleCount: 1, + impactedCount: 8, + unusedAxes: truncatedAxes, + }); + expect(scored.risk).toBe('UNKNOWN'); + expect(scored.riskScale.comparableAcrossKinds).toBe(false); + }); +}); + +describe('unusedAxesForImpactWalk', () => { + it('marks File, skipEnrichment, zero budget, and query failure distinctly', () => { + expect( + unusedAxesForImpactWalk({ + isFileTarget: true, + skipEnrichment: false, + maxChunks: 10, + processQueryFailed: true, + moduleQueryFailed: true, + impactedCount: 1, + }).every((a) => a.reason === 'file-nodes-have-no-process-or-community-membership'), + ).toBe(true); + expect( + unusedAxesForImpactWalk({ + isFileTarget: false, + skipEnrichment: true, + maxChunks: 0, + processQueryFailed: false, + moduleQueryFailed: false, + impactedCount: 1, + }).map((a) => a.reason), + ).toEqual(['enrichment-skipped', 'enrichment-skipped']); + expect( + unusedAxesForImpactWalk({ + isFileTarget: false, + skipEnrichment: false, + maxChunks: 0, + processQueryFailed: false, + moduleQueryFailed: false, + impactedCount: 1, + }).map((a) => a.reason), + ).toEqual(['enrichment-budget-exhausted', 'enrichment-budget-exhausted']); + expect( + unusedAxesForImpactWalk({ + isFileTarget: false, + skipEnrichment: false, + maxChunks: 10, + processQueryFailed: true, + moduleQueryFailed: false, + impactedCount: 1, + }), + ).toEqual([{ axis: 'processes', reason: 'enrichment-query-failed' }]); + expect( + unusedAxesForImpactWalk({ + isFileTarget: false, + skipEnrichment: false, + maxChunks: 0, + processQueryFailed: false, + moduleQueryFailed: false, + impactedCount: 0, + }), + ).toEqual([]); + expect( + unusedAxesForImpactWalk({ + isFileTarget: false, + skipEnrichment: false, + maxChunks: 10, + processQueryFailed: false, + moduleQueryFailed: false, + impactedCount: 501, + enrichmentTruncated: true, + }).map((a) => a.reason), + ).toEqual(['enrichment-truncated', 'enrichment-truncated']); + }); +}); diff --git a/gitnexus/test/unit/shipped-skills-sync.test.ts b/gitnexus/test/unit/shipped-skills-sync.test.ts index c82e45321..984037e4c 100644 --- a/gitnexus/test/unit/shipped-skills-sync.test.ts +++ b/gitnexus/test/unit/shipped-skills-sync.test.ts @@ -170,6 +170,21 @@ describe('intended standard-skill improvements stay in every applicable copy', ( } }); + it('keeps the cross-surface risk-scale guidance in every impact-analysis copy', () => { + const required = [ + '`riskSharedAxes`', + 'MCP File walks', + 'web Graph-RAG expands File targets', + 'Within single-repo mode', + 'Within group mode', + 'overlays resolved', + ]; + for (const file of standardSkillCopies('gitnexus-impact-analysis')) { + const content = fs.readFileSync(file, 'utf-8'); + for (const fragment of required) expect(content).toContain(fragment); + } + }); + // Same shape as the UNKNOWN guard above, for the other half of the verdict: // `detect_changes` can come back SHORT — `partial` when a batched graph query // failed, `truncated` when the changed-symbol listing hit its cap — and both @@ -331,6 +346,10 @@ describe('root AGENTS.md / CLAUDE.md managed block keeps the risk: UNKNOWN polic const REQUIRED_FRAGMENTS = [ 'MUST treat `risk: UNKNOWN` as unresolved, not as low.', 'never read `UNKNOWN` as an all-clear', + 'never use `riskSharedAxes` to waive a HIGH/CRITICAL `risk` warning', + 'Compare File/symbol', + 'MCP File omits axes', + 'Graph-RAG expands File', ]; it.each(['AGENTS.md', 'CLAUDE.md'])('%s managed block documents the policy', (file) => { diff --git a/gitnexus/test/unit/tools.test.ts b/gitnexus/test/unit/tools.test.ts index d9a2890d9..700407f78 100644 --- a/gitnexus/test/unit/tools.test.ts +++ b/gitnexus/test/unit/tools.test.ts @@ -193,6 +193,17 @@ describe('GITNEXUS_TOOLS', () => { expect(impactTool.description).toContain('truncatedBy'); }); + it('documents riskSharedAxes as a compare aid, not the edit gate', () => { + const impactTool = GITNEXUS_TOOLS.find((t) => t.name === 'impact')!; + expect(impactTool.description).toContain('riskSharedAxes'); + expect(impactTool.description).toContain('Never substitute it for `risk`'); + expect(impactTool.description).toContain('IMPACT_MAX_CHUNKS=0'); + expect(impactTool.description).toContain('sampled a subset of impacted symbols'); + expect(impactTool.description).toContain('Graph-RAG'); + expect(impactTool.description).toContain('cross-repo crossing overlay'); + expect(impactTool.description).toContain('known HIGH/CRITICAL warnings survive'); + }); + it.each(['query', 'context', 'impact'])( '%s advertises an optional positive maxTokens budget', (name) => {