Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand All @@ -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))
Expand Down Expand Up @@ -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 ""
Expand Down
54 changes: 54 additions & 0 deletions tests/unit/server/mcp/test_risk.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading