diff --git a/packages/server/src/repowise/server/mcp_server/tool_risk/assessment.py b/packages/server/src/repowise/server/mcp_server/tool_risk/assessment.py index da82ce77b..22640a0b7 100644 --- a/packages/server/src/repowise/server/mcp_server/tool_risk/assessment.py +++ b/packages/server/src/repowise/server/mcp_server/tool_risk/assessment.py @@ -5,6 +5,7 @@ import contextlib import json from datetime import UTC, datetime +from pathlib import Path from typing import Any from sqlalchemy import select, text @@ -29,6 +30,37 @@ _TOP_FIX_SYMBOLS = 3 +def normalize_target_path(target: str, repo_root: str | None = None) -> str: + """Normalize a caller-supplied file path to the POSIX-relative form stored + in ``git_metadata.file_path``. + + ``get_risk`` matches ``file_path`` by exact string equality, but callers + reach it through git tools, shell completion, or editors that hand over a + backslash form (Windows), a leading ``./``, an absolute path, or a trailing + separator. Any of those makes the row lookup miss, and ``_assess_one_target`` + then reports the indistinguishable ``no git metadata available`` card + (hotspot_score=0, primary_owner=None, empty co_change_partners) even though + the row exists — issue #1279. Normalizing the caller's side closes that gap. + """ + normalized = target.replace("\\", "/") + # Make a repo-absolute path (``/abs/repo/src/x.py``) relative to the repo + # root when we know it. Uses a prefix check on the normalized forms, so a + # path that is already repo-relative is left untouched. + if repo_root: + root_norm = str(Path(repo_root).resolve()).replace("\\", "/") + try: + resolved = Path(normalized).resolve() + if str(resolved).startswith(root_norm.rstrip("/") + "/"): + normalized = str(resolved).replace("\\", "/")[len(root_norm.rstrip("/")) + 1 :] + except OSError: + pass + # Strip a leading cwd-relative prefix and any leading slash left over. + normalized = normalized.lstrip("/").lstrip(".") + # Collapse duplicate slashes and any trailing separator. + parts = [p for p in normalized.split("/") if p] + return "/".join(parts) + + def _derive_change_pattern(categories: dict[str, int]) -> str: """Derive a human-readable change pattern from commit category counts.""" if not categories: @@ -400,13 +432,24 @@ async def _assess_one_target( repo_id = repository.id result_data: dict[str, Any] = {"target": target} - dep_count = all_edge_map.get(target, 0) + # Callers reach get_risk with the file path in many forms — backslashes + # (Windows), a leading ``./``, a trailing separator, or a repo-absolute + # path — while git_metadata.file_path (and the graph node/edge ids) are + # stored POSIX-relative. Exact-string equality against the raw target made + # a row that exists look absent, and _assess_one_target then reported the + # indistinguishable "no git metadata available" card (hotspot_score=0, + # primary_owner=None, empty co_change_partners) — issue #1279. Normalize + # once and key every file-path lookup on it, but keep the response keyed by + # what the caller asked for. + lookup_path = normalize_target_path(target, repo_root=repository.local_path) + + dep_count = all_edge_map.get(lookup_path, 0) # Git metadata res = await session.execute( select(GitMetadata).where( GitMetadata.repository_id == repo_id, - GitMetadata.file_path == target, + GitMetadata.file_path == lookup_path, ) ) meta = res.scalar_one_or_none() @@ -420,19 +463,19 @@ async def _assess_one_target( result_data["trend"] = "unknown" result_data["risk_type"] = "high-coupling" if dep_count >= 5 else "unknown" result_data["impact_surface"] = _compute_impact_surface( - target, + lookup_path, reverse_deps, node_meta, exclude_spec, ) - result_data["test_gap"] = await _check_test_gap(session, repo_id, target) - result_data["security_signals"] = await _get_security_signals(session, repo_id, target) + result_data["test_gap"] = await _check_test_gap(session, repo_id, lookup_path) + result_data["security_signals"] = await _get_security_signals(session, repo_id, lookup_path) result_data["risk_summary"] = f"{target} — no git metadata available" return result_data hotspot_score = meta.churn_percentile or 0.0 - co_changes = _build_co_changes(meta, import_links.get(target, set()), exclude_spec) + co_changes = _build_co_changes(meta, import_links.get(lookup_path, set()), exclude_spec) owner = meta.primary_owner_name or "unknown" pct = meta.primary_owner_commit_pct or 0.0 @@ -444,7 +487,7 @@ async def _assess_one_target( risk_type = _classify_risk_type(meta, dep_count, team_size) # --- Impact surface --- - impact_surface = _compute_impact_surface(target, reverse_deps, node_meta, exclude_spec) + impact_surface = _compute_impact_surface(lookup_path, reverse_deps, node_meta, exclude_spec) # Phase 2: commit classification → change_pattern change_pattern = _derive_change_pattern(_load_commit_categories(meta)) @@ -483,8 +526,8 @@ async def _assess_one_target( result_data["defect_profile"] = defect_profile # C. Test gaps + security signals - result_data["test_gap"] = await _check_test_gap(session, repo_id, target) - result_data["security_signals"] = await _get_security_signals(session, repo_id, target) + result_data["test_gap"] = await _check_test_gap(session, repo_id, lookup_path) + result_data["security_signals"] = await _get_security_signals(session, repo_id, lookup_path) capped = getattr(meta, "commit_count_capped", False) capped_note = " (history truncated — actual count may be higher)" if capped else "" diff --git a/tests/unit/server/mcp/test_risk.py b/tests/unit/server/mcp/test_risk.py index 23dc953bd..e99196fda 100644 --- a/tests/unit/server/mcp/test_risk.py +++ b/tests/unit/server/mcp/test_risk.py @@ -65,6 +65,43 @@ async def test_get_risk_global_hotspots_exclude_targets(setup_mcp): assert h["file_path"] != "src/auth/service.py" +@pytest.mark.asyncio +async def test_get_risk_normalizes_target_path(setup_mcp): + """#1279: git_metadata row lookup must survive non-POSIX target forms. + + Callers hand paths over with backslashes, a leading ``./``, or a trailing + separator. get_risk matches git_metadata.file_path by exact equality, so + each of these previously missed the row and reported the "no git metadata + available" card (hotspot_score=0 / primary_owner=None / empty partners) + even though the row exists. + """ + from repowise.server.mcp_server import get_risk + + for target in ("src\\auth\\service.py", "./src/auth/service.py", "src/auth/service.py/"): + result = await get_risk([target]) + # Response stays keyed by the caller's exact string. + t = result["targets"][target] + assert t["hotspot_score"] == 0.92, target + assert t["primary_owner"] == "Alice", target + assert len(t["co_change_partners"]) == 2, target + assert "no git metadata available" not in t["risk_summary"], target + # Trend: 30d=3, 90d=8 → stable. + assert t["trend"] == "stable", target + + +@pytest.mark.asyncio +async def test_get_risk_repo_absolute_target_path(setup_mcp): + """A repo-absolute target is made repo-relative before the lookup (#1279).""" + from repowise.server.mcp_server import get_risk + + abs_target = "/tmp/test-repo/src/auth/service.py" + result = await get_risk([abs_target]) + t = result["targets"][abs_target] + assert t["hotspot_score"] == 0.92 + assert t["primary_owner"] == "Alice" + assert len(t["co_change_partners"]) == 2 + + @pytest.mark.asyncio async def test_get_risk_no_git_metadata(setup_mcp): from repowise.server.mcp_server import get_risk @@ -79,6 +116,23 @@ async def test_get_risk_no_git_metadata(setup_mcp): assert "impact_surface" in t +@pytest.mark.parametrize( + ("raw", "repo_root", "expected"), + [ + ("src/auth/service.py", None, "src/auth/service.py"), + ("src\\auth\\service.py", None, "src/auth/service.py"), + ("./src/auth/service.py", None, "src/auth/service.py"), + ("src/auth/service.py/", None, "src/auth/service.py"), + ("src//auth//service.py", None, "src/auth/service.py"), + ("/tmp/test-repo/src/auth/service.py", "/tmp/test-repo", "src/auth/service.py"), + ], +) +def test_normalize_target_path(raw, repo_root, expected): + from repowise.server.mcp_server.tool_risk.assessment import normalize_target_path + + assert normalize_target_path(raw, repo_root=repo_root) == expected + + @pytest.mark.asyncio async def test_get_risk_stable_file(setup_mcp): from repowise.server.mcp_server import get_risk