Skip to content

fix(risk): normalize the git_metadata target path in get_risk - #1849

Open
sloemo01 wants to merge 1 commit into
repowise-dev:mainfrom
sloemo01:fix/get_risk-colnames
Open

fix(risk): normalize the git_metadata target path in get_risk#1849
sloemo01 wants to merge 1 commit into
repowise-dev:mainfrom
sloemo01:fix/get_risk-colnames

Conversation

@sloemo01

Copy link
Copy Markdown
Contributor

Summary

Fixes #1279: get_risk returned hotspot_score=0, primary_owner=null, co_change_partners=[], and a "no git metadata available" summary for files that do have rich git_metadata rows.

Root cause

_assess_one_target looked up the GitMetadata row by exact string equality on file_path against the caller's raw target. The index stores file_path POSIX-relative (src/auth/service.py), but callers reach get_risk through git tools, shell completion, or editors that hand over:

  • backslash form (src\auth\service.py, common on Windows),
  • a leading ./,
  • a trailing separator,
  • or a repo-absolute path (/abs/repo/src/auth/service.py).

Any of these made the lookup miss, so meta was None and _assess_one_target emitted the indistinguishable "no git metadata available" card — the exact payload the reporter saw. (The v0.37 column names were already correct; the lookup simply never found the row.)

Change

  • Add normalize_target_path(target, repo_root) in assessment.py: converts backslashes to /, strips a leading ./ and any leading slash, collapses duplicate/trailing separators, and makes a repo-absolute path relative to repository.local_path.
  • In _assess_one_target, normalize the target once and key every file-path lookup (git_metadata, graph edges/nodes, test gap, security signals) on the normalized form, while keeping the response keyed by the caller's exact string.

Tests

Added regression tests in tests/unit/server/mcp/test_risk.py:

  • test_get_risk_normalizes_target_path — backslash, ./, trailing-slash forms now populate hotspot_score/primary_owner/co_change_partners.
  • test_get_risk_repo_absolute_target_path — repo-absolute form resolves.
  • test_normalize_target_path — parametrized unit coverage of the helper.

Ruff + all test_risk.py / test_risk_dependency_edges.py tests pass.

…n get_risk

get_risk matches git_metadata.file_path by exact string equality, but callers
hand the path over in many forms — backslashes (Windows), a leading ./,
a trailing separator, or a repo-absolute path — while file_path is stored
POSIX-relative. Any of those made the row lookup miss, and _assess_one_target
then reported the indistinguishable "no git metadata available" card
(hotspot_score=0, primary_owner=None, empty co_change_partners) even though
the row exists (issue repowise-dev#1279).

Normalize the target once in _assess_one_target and key every file-path
lookup (git_metadata, graph edges/nodes, test gap, security signals) on the
normalized form, while keeping the response keyed by the caller's exact
string. Add a regression test covering backslash, ./, trailing-slash,
duplicate-slash, and repo-absolute forms.
@repowise-bot

repowise-bot Bot commented Aug 22, 2026

Copy link
Copy Markdown

✅ Health of changed files: 2.9 → 3.2 (+0.3)
⚠️ Change risk: moderate, riskier than 35% of this repo's commits.

📋 At a glance
1 file changed health · 2 hotspots touched · 1 new finding introduced · 2 files with recent fix history.

Files & modules (2)
  • packages (1 file)
    • .../tool_risk/assessment.py
  • tests (1 file)
    • .../mcp/test_risk.py

✅ Health gate: passed

📌 Before you merge

  • Run .../persistence/test_test_gap_graph_floor.py, .../mcp/test_defect_profile.py, .../mcp/test_payload_cuts.py: they import the changed files
🔎 More signals (2)

🗺️ Change map

flowchart LR
  subgraph PR ["Changed in this PR (1 with dependents)"]
    f_packages_server_src_repowise_server_mcp_server_tool_risk_assessment_py[".../tool_risk/assessment.py 🔥"]:::changed
  end
  f_packages_server_src_repowise_server_mcp_server_tool_context_targets_py[".../tool_context/targets.py"]
  f_packages_server_src_repowise_server_mcp_server_tool_risk_assessment_py --> f_packages_server_src_repowise_server_mcp_server_tool_context_targets_py
  f_packages_server_src_repowise_server_mcp_server_tool_risk___init___py[".../tool_risk/__init__.py"]
  f_packages_server_src_repowise_server_mcp_server_tool_risk_assessment_py --> f_packages_server_src_repowise_server_mcp_server_tool_risk___init___py
  f_packages_server_src_repowise_server_mcp_server_tool_risk_get_risk_py[".../tool_risk/get_risk.py"]
  f_packages_server_src_repowise_server_mcp_server_tool_risk_assessment_py --> f_packages_server_src_repowise_server_mcp_server_tool_risk_get_risk_py
  t_tests_unit_persistence_test_test_gap_graph_floor_py(["✅ .../persistence/test_test_gap_graph_floor.py"]):::guard
  t_tests_unit_persistence_test_test_gap_graph_floor_py -.-> f_packages_server_src_repowise_server_mcp_server_tool_risk_assessment_py
  classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
  classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
  classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Loading

Solid arrows: code that imports the changed files (3 direct dependents, from the last indexed snapshot). Dashed: history/tests.

🔥 Hotspots touched (2)

  • .../mcp/test_risk.py: 7 commits/90d, 2 dependents · primary owner: Raghav Chamadiya (74%)
  • .../tool_risk/assessment.py: 10 commits/90d, 7 dependents · primary owner: Raghav Chamadiya (98%)

👀 Suggested reviewers @RaghavChamadiya


📊 See the full report for this PR
Your repo map with this PR's blast radius lit up, every caller of the contracts it changes, and health before and after. No sign-in. · ⭐ Star Repowise · 📥 Install bot · Silence on a single PR with [skip repowise] in the title · Per-repo toggle on repowise.dev/settings?tab=bot · Updated 2026-08-22 14:35 UTC

@Ayush7614 Ayush7614 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified: packages/server/src/repowise/server/mcp_server/tool_risk/assessment.py:1 adds normalize_target_path handling backslash, ./, trailing slash, repo-absolute → POSIX-relative, fixing #1279 where exact-string lookup returned hotspot_score=0. All lookups now use normalized path while response keeps raw target. Parametrized tests in test_risk.py cover forms. Correct fix — LGTM.

@RaghavChamadiya

Copy link
Copy Markdown
Member

Thanks @sloemo01. The framing is right: a caller-side path that misses the row produces the no git metadata available card, which is indistinguishable from a repo with no history, so the user has no way to tell a normalization miss from a real absence. Normalizing on our side rather than loosening the match is the right direction.

One line in normalize_target_path is wrong, and it fails on paths this repo is full of:

normalized = normalized.lstrip("/").lstrip(".")

str.lstrip takes a set of characters, not a prefix, so it removes every leading dot, not a leading ./. I ran it:

'./src/x.py'               -> 'src/x.py'          correct
'.github/workflows/ci.yml' -> 'github/workflows/ci.yml'
'.claude/TRIAGE.md'        -> 'claude/TRIAGE.md'

Every dot-directory loses its dot, so get_risk on .github/workflows/ci.yml now looks up github/workflows/ci.yml, which matches nothing, and reports the same empty card the issue is about. The function makes the reported failure worse for dotfiles while fixing it for backslashes.

A prefix strip is what you want:

if normalized.startswith("./"):
    normalized = normalized[2:]
normalized = normalized.lstrip("/")

Two smaller things in the same function, both non-blocking:

The Path(normalized).resolve() inside the repo_root branch resolves against the process cwd, which for the MCP server is not the repo. A relative path that happens to exist under the server's cwd could resolve somewhere unrelated and then fail the prefix check for the wrong reason. Since you already have repo_root, Path(repo_root, normalized).resolve() for the relative case says what you mean. And except OSError will not catch the ValueError that resolve() can raise on a malformed Windows path, which is exactly the platform this is written for.

The rest of the PR is good and the trailing-separator and duplicate-slash handling is worth having. Push the prefix strip and I will take it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v0.37.0 get_risk returns hotspot_score=0, primary_owner=null despite data in wiki.db — column name mismatch

3 participants