Skip to content

Commit faa8042

Browse files
fix(mcp): rank get_risk's attention list and get_context's triage on fix history (#956)
get_risk classified a target "bug-prone" from counted fixes, then showed an attention list beside it ranked purely on churn, so the two halves of one response disagreed about what deserves attention. get_context's triage card had the churn bit alone and no defect signal at all. global_hotspots now admits bug magnets rather than filtering on is_hotspot, and orders by fix history before churn. The filter matters as much as the order: is_hotspot-only means a file fixed four times last month that is not busy can never appear, so no amount of re-sorting inside the churn set would reach it. Churn stays the fallback, and rows are annotated with the fix count and age only when there is fix history. These are full ORM rows, so no query is added. risk_summary leads with the fix history when a file has any, so the first thing read agrees with the risk_type printed on the same line. Files with no fixes keep the summary they had. get_context gains a fix_history triage pointer (count, age, magnet flag) from the same single row, widened from one column to four. Omitted entirely on files with no counted fixes, so a repo without fix history pays nothing. The recency contract now lives in one place: fix_annotation never emits the bug_magnet flag without an age to anchor it, and _defect_profile is built on it rather than repeating the rule. Cost: ~73 tokens worst case per get_risk response with all five rows carrying fix data, ~19 per get_context file target, zero without fix history.
1 parent f000d4c commit faa8042

6 files changed

Lines changed: 328 additions & 31 deletions

File tree

packages/server/src/repowise/server/mcp_server/tool_context/context.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ async def get_context(
5656
5757
Returns title, summary, signatures, hotspot bit, decision_record titles,
5858
and symbol_ids to pipe into get_symbol (cheaper than Read for bodies).
59+
fix_history appears only on files with counted bug fixes (count, age,
60+
bug_magnet); hotspot is churn. Either one is a cue to call get_risk.
5961
Batch targets in one call. File targets above ~80 lines default to a
6062
skeleton (every signature + top-PageRank bodies, with a verified flag —
6163
a fraction of Read cost); ``mostly_full`` marks files where a direct

packages/server/src/repowise/server/mcp_server/tool_context/targets.py

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
_find_layer_for_file,
4646
_find_tour_step_for_file,
4747
)
48+
from repowise.server.mcp_server.tool_risk.assessment import fix_annotation
4849

4950

5051
# Skeleton-by-default threshold for file targets. Measured on this repo: a
@@ -555,6 +556,12 @@ async def _resolve_one_target(
555556
# * ``hotspot``: lights the way to ``get_risk`` for files in the 95th+
556557
# churn percentile. Just the boolean — the full risk dossier stays
557558
# in ``get_risk`` so the triage card doesn't grow.
559+
# * ``fix_history``: the same pointer for the defect signal, and the
560+
# reason the card grew by one key. ``hotspot`` alone answers "is this
561+
# file busy", which is not the same question as "does this file break",
562+
# and get_risk already classifies targets bug-prone off these columns.
563+
# Count plus age plus the magnet flag, no symbols and no dossier;
564+
# omitted entirely on files with no counted fixes.
558565
# * ``decision_records``: titles only, no body. Lights the way to
559566
# ``get_why``. We deliberately don't inline the rationale here;
560567
# duplicating it across every ``get_context`` response bloats the
@@ -565,14 +572,26 @@ async def _resolve_one_target(
565572
if target_type == "module" and page:
566573
triage_path = page.target_path
567574
if triage_path:
575+
# Same single row, four columns instead of one: no extra round trip.
568576
triage_meta_res = await session.execute(
569-
select(GitMetadata.is_hotspot).where(
577+
select(
578+
GitMetadata.is_hotspot,
579+
GitMetadata.prior_defect_count,
580+
GitMetadata.bug_magnet,
581+
GitMetadata.last_fix_at,
582+
).where(
570583
GitMetadata.repository_id == repo_id,
571584
GitMetadata.file_path == triage_path,
572585
)
573586
)
574-
triage_meta = triage_meta_res.scalar_one_or_none()
575-
result_data["hotspot"] = bool(triage_meta) if triage_meta is not None else False
587+
triage_meta = triage_meta_res.one_or_none()
588+
result_data["hotspot"] = bool(triage_meta.is_hotspot) if triage_meta is not None else False
589+
if triage_meta is not None:
590+
# Row exposes the selected columns as attributes, which is exactly
591+
# the shape fix_annotation reads off a full ORM row.
592+
fixes = fix_annotation(triage_meta)
593+
if fixes is not None:
594+
result_data["fix_history"] = fixes
576595

577596
# Governing decisions — opt-in only (``include=["decisions"]``).
578597
# The default triage card omits them: the rich form

packages/server/src/repowise/server/mcp_server/tool_risk/assessment.py

Lines changed: 53 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,49 @@ def _build_co_changes(meta: Any, import_related: set[str], exclude_spec: Any) ->
262262
)
263263

264264

265+
def fix_annotation(meta: Any) -> dict | None:
266+
"""Counted fixes, their age, and the magnet flag, or ``None`` for silence.
267+
268+
The compact form every fix-history surface shares, so the recency contract
269+
is enforced once: the ``bug_magnet`` flag rides on the age and is never
270+
emitted alone. ``bug_magnet`` is a claim about RECENT fix pressure, so with
271+
no timestamp to anchor it the same word would describe a file fixed four
272+
times last month and one fixed four times two years ago.
273+
274+
Read off the ``GitMetadata`` row the caller already loaded: no query.
275+
"""
276+
count = getattr(meta, "prior_defect_count", 0) or 0
277+
if count <= 0:
278+
return None
279+
280+
out: dict[str, Any] = {"fix_count": count}
281+
last_fix_at = getattr(meta, "last_fix_at", None)
282+
if isinstance(last_fix_at, datetime):
283+
# Rows are stored naive-UTC; compare on the same footing.
284+
moment = last_fix_at if last_fix_at.tzinfo else last_fix_at.replace(tzinfo=UTC)
285+
out["last_fix_days_ago"] = max(0, (datetime.now(UTC) - moment).days)
286+
if getattr(meta, "bug_magnet", False):
287+
out["bug_magnet"] = True
288+
return out
289+
290+
291+
def _fix_clause(profile: dict | None) -> str:
292+
"""Lead clause for ``risk_summary``, or empty when there is no fix history.
293+
294+
Trailing separator included so the caller concatenates without a dangling
295+
comma on files that have never been fixed. Never renders a count without an
296+
age: an unanchored count reads as a claim about the distant past.
297+
"""
298+
if not profile or "last_fix_days_ago" not in profile:
299+
return ""
300+
n = profile["fix_count"]
301+
magnet = " (bug magnet)" if profile.get("bug_magnet") else ""
302+
return (
303+
f"{n} bug fix{'es' if n != 1 else ''} in 6mo, "
304+
f"last {profile['last_fix_days_ago']}d ago{magnet}, "
305+
)
306+
307+
265308
def _defect_profile(meta: Any) -> dict | None:
266309
"""What this file's counted bug fixes say about it, or ``None`` for silence.
267310
@@ -279,23 +322,10 @@ def _defect_profile(meta: Any) -> dict | None:
279322
string repeated once per target is exactly the per-file cost the lean-MCP
280323
work went to some trouble to remove.
281324
"""
282-
count = getattr(meta, "prior_defect_count", 0) or 0
283-
if count <= 0:
325+
profile = fix_annotation(meta)
326+
if profile is None:
284327
return None
285-
286-
profile: dict[str, Any] = {"fix_count": count, "window": "6 months"}
287-
288-
last_fix_at = getattr(meta, "last_fix_at", None)
289-
if isinstance(last_fix_at, datetime):
290-
# Rows are stored naive-UTC; compare on the same footing.
291-
moment = last_fix_at if last_fix_at.tzinfo else last_fix_at.replace(tzinfo=UTC)
292-
profile["last_fix_days_ago"] = max(0, (datetime.now(UTC) - moment).days)
293-
# The flag rides on the age, never alone. bug_magnet is a claim about
294-
# RECENT fix pressure, so without a timestamp to anchor it the same
295-
# word would describe a file fixed four times last month and one fixed
296-
# four times two years ago.
297-
if getattr(meta, "bug_magnet", False):
298-
profile["bug_magnet"] = True
328+
profile["window"] = "6 months"
299329

300330
symbols = _top_fix_symbols(getattr(meta, "fix_symbol_counts_json", None))
301331
if symbols:
@@ -449,8 +479,14 @@ async def _assess_one_target(
449479
# later by cross-repo enrichment. We store dep_count now and let the
450480
# outer function rebuild the summary after enrichment if needed.
451481
result_data["_base_dep_count"] = dep_count
482+
# Lead with the bug-fix history when there is any. The summary used to open
483+
# on a churn percentile even where risk_type said "bug-prone", so the first
484+
# thing an agent read disagreed with the classification beside it. Counted
485+
# fixes are the better grounded defect signal, so they go first and churn
486+
# keeps its place as the next clause.
452487
result_data["risk_summary"] = (
453-
f"{target} — hotspot score {hotspot_score:.0%} ({trend}), "
488+
f"{target}{_fix_clause(defect_profile)}"
489+
f"hotspot score {hotspot_score:.0%} ({trend}), "
454490
f"{dep_count} dependents, {risk_type}, {change_pattern}, "
455491
f"{len(co_changes)} co-change partners, owned {pct:.0%} by {owner}"
456492
f"{bus_note}{capped_note}"

packages/server/src/repowise/server/mcp_server/tool_risk/get_risk.py

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
)
2525
from repowise.server.mcp_server._meta import build_meta as _build_meta
2626

27-
from .assessment import _assess_one_target, _get_active_contributor_count
27+
from .assessment import _assess_one_target, _get_active_contributor_count, fix_annotation
2828
from .directives import _build_pr_directive, _governance_directive
2929
from .enrichment import _enrich_cross_repo, _enrich_health, _finalize_dep_summaries
3030

@@ -35,11 +35,11 @@ async def get_risk(
3535
repo: str | None = None,
3636
changed_files: list[str] | None = None,
3737
) -> dict:
38-
"""What history says about touching these files — churn, owners, blast radius.
38+
"""What history says about touching these files — bug fixes, churn, owners.
3939
4040
Fuses git temporal signals (churn percentile, trend, bus factor) with
4141
graph topology (dependents, co-changes, impact surface) and security
42-
findings. Consult before editing 95th+ churn-percentile files. Pass
42+
findings. Consult before editing a file that is bug-fixed or busy. Pass
4343
changed_files for PR mode: the response leads with a directive block
4444
(will_break, missing_cochanges, missing_tests, tests_to_run) — read it
4545
first. tests_to_run is coverage-backed: the tests the per-test map proves
@@ -53,6 +53,7 @@ async def get_risk(
5353
are approximate, because symbol spans are current-tree while each fix's line
5454
ranges are numbered on its own parent commit, so read them as "mostly here"
5555
rather than exact. Nothing here names the commit that introduced a bug.
56+
global_hotspots ranks the same way: fix history first, churn as fallback.
5657
5758
Args:
5859
targets: file paths to assess.
@@ -115,27 +116,48 @@ async def get_risk(
115116
]
116117
)
117118

118-
# Global hotspots (excluding requested targets)
119+
# Elsewhere-in-the-repo attention list (excluding requested targets).
120+
# Ranked on bug-fix history first, churn second. This list sits beside
121+
# per-target verdicts that already read "bug-prone" off counted fixes,
122+
# so ranking it purely on churn made the two halves of one response
123+
# disagree about what deserves attention. Admitting bug magnets matters
124+
# as much as the ordering: filtering on is_hotspot alone means a file
125+
# fixed four times last month that is not busy can never appear.
126+
# Churn stays the fallback, so a repo with no fix convention keeps
127+
# exactly the list it had. These are full ORM rows, so the fix columns
128+
# are already in memory and this adds no query.
119129
target_set = set(targets)
120130
res = await session.execute(
121131
select(GitMetadata)
122132
.where(
123133
GitMetadata.repository_id == repo_id,
124-
GitMetadata.is_hotspot == True, # noqa: E712
134+
(GitMetadata.is_hotspot == True) # noqa: E712
135+
| (GitMetadata.bug_magnet == True), # noqa: E712
136+
)
137+
.order_by(
138+
GitMetadata.bug_magnet.desc(),
139+
GitMetadata.fix_mass.desc(),
140+
GitMetadata.churn_percentile.desc(),
125141
)
126-
.order_by(GitMetadata.churn_percentile.desc())
127142
.limit(len(targets) + 5)
128143
)
129144
all_hotspots = filter_rows_by_attr(list(res.scalars().all()), "file_path", exclude_spec)
130-
global_hotspots = [
131-
{
145+
global_hotspots = []
146+
for h in all_hotspots:
147+
if h.file_path in target_set:
148+
continue
149+
entry = {
132150
"file_path": h.file_path,
133151
"hotspot_score": h.churn_percentile,
134152
"primary_owner": h.primary_owner_name,
135153
}
136-
for h in all_hotspots
137-
if h.file_path not in target_set
138-
][:5]
154+
# Silent on files with no counted fixes, so a repo without fix
155+
# history pays nothing for this.
156+
fixes = fix_annotation(h)
157+
if fixes is not None:
158+
entry.update(fixes)
159+
global_hotspots.append(entry)
160+
global_hotspots = global_hotspots[:5]
139161

140162
# A. PR blast radius (only when caller passes changed_files)
141163
pr_blast_radius: dict | None = None

tests/unit/server/mcp/test_defect_profile.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
from repowise.server.mcp_server.tool_risk.assessment import (
1919
_classify_risk_type,
2020
_defect_profile,
21+
_fix_clause,
2122
_top_fix_symbols,
23+
fix_annotation,
2224
)
2325

2426

@@ -132,3 +134,64 @@ async def test_get_risk_omits_the_block_without_fix_data(setup_mcp):
132134

133135
result = await get_risk(["src/auth/service.py"])
134136
assert "defect_profile" not in result["targets"]["src/auth/service.py"]
137+
138+
139+
# ---------------------------------------------------------------------------
140+
# The shared fix annotation, and the risk_summary clause built on it
141+
# ---------------------------------------------------------------------------
142+
143+
144+
def test_fix_annotation_is_silent_without_counted_fixes():
145+
assert fix_annotation(_meta()) is None
146+
147+
148+
def test_fix_annotation_withholds_the_magnet_flag_without_an_age():
149+
# bug_magnet is a claim about RECENT fix pressure. With no timestamp the
150+
# same word would describe a file fixed four times last month and one
151+
# fixed four times two years ago, so the flag drops rather than mislead.
152+
out = fix_annotation(_meta(prior_defect_count=9, bug_magnet=True, last_fix_at=None))
153+
assert out == {"fix_count": 9}
154+
assert "bug_magnet" not in out
155+
156+
157+
def test_fix_annotation_carries_count_age_and_flag():
158+
out = fix_annotation(
159+
_meta(
160+
prior_defect_count=5,
161+
bug_magnet=True,
162+
last_fix_at=datetime.now(UTC) - timedelta(days=14),
163+
)
164+
)
165+
assert out == {"fix_count": 5, "last_fix_days_ago": 14, "bug_magnet": True}
166+
167+
168+
def test_defect_profile_still_builds_on_the_shared_annotation():
169+
# The profile is the annotation plus a window and symbols, so the recency
170+
# contract is enforced in exactly one place.
171+
profile = _defect_profile(
172+
_meta(
173+
prior_defect_count=2,
174+
bug_magnet=True,
175+
last_fix_at=datetime.now(UTC) - timedelta(days=3),
176+
)
177+
)
178+
assert profile["fix_count"] == 2
179+
assert profile["last_fix_days_ago"] == 3
180+
assert profile["bug_magnet"] is True
181+
assert profile["window"] == "6 months"
182+
183+
184+
def test_risk_summary_clause_is_empty_without_fix_history():
185+
# Files that have never been fixed must not gain a dangling separator.
186+
assert _fix_clause(None) == ""
187+
assert _fix_clause({"fix_count": 4}) == "" # count with no age
188+
189+
190+
def test_risk_summary_clause_leads_with_fixes_and_closes_its_separator():
191+
clause = _fix_clause({"fix_count": 5, "last_fix_days_ago": 14, "bug_magnet": True})
192+
assert clause == "5 bug fixes in 6mo, last 14d ago (bug magnet), "
193+
194+
195+
def test_risk_summary_clause_singularizes_one_fix():
196+
clause = _fix_clause({"fix_count": 1, "last_fix_days_ago": 2})
197+
assert clause == "1 bug fix in 6mo, last 2d ago, "

0 commit comments

Comments
 (0)