Skip to content

Commit a3ae800

Browse files
authored
feat(risk): split test-file breakage out of will_break (#672) (#739)
* feat(risk): split test-file breakage out of will_break (#672) In get_risk PR mode, the will_break directive list mixed production and test files, so a burst of broken tests could crowd real structural impact out of the capped list. Segment test files into a new will_break_tests field. The split reuses the is_test flag already loaded into node_meta, partitioning before the per-list cap so production impact keeps its full budget. The summary now reports the test-breakage count alongside downstream impact. Closes #672 * docs(mcp): document will_break_tests and the local get_risk directive fields (#672)
1 parent beb6499 commit a3ae800

5 files changed

Lines changed: 59 additions & 3 deletions

File tree

docs/MCP_TOOLS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ Modification risk assessment for files or a set of changed files.
313313

314314
**Returns:** Per-file risk score (0-10), hotspot status, dependent count, co-change partners, blast radius, recommended reviewers, test gap analysis, security signals. In workspace mode, enriched with cross-repo co-change partners and contract dependencies.
315315

316-
When `changed_files` is passed, the response leads with a `directive` block. In workspace mode that directive also carries the cross-repo fallout of the changed repo:
316+
When `changed_files` is passed, the response leads with a `directive` block. Its core lists are the local blast radius: `will_break` (production files that depend on the diff and are likely to break), `will_break_tests` (test files impacted the same way, kept separate so a burst of broken tests doesn't crowd production impact out of the capped list), `missing_cochanges` (historical co-changers absent from the diff), and `missing_tests` (changed files without test coverage). In workspace mode that directive also carries the cross-repo fallout of the changed repo:
317317

318318
- `will_break_consumers`: services in *other* repos that depend on this one (structural impact), each with `repo`, `service`, `distance`, `score`, and the edge kinds carrying the impact.
319319
- `missing_cross_repo_cochanges`: services in other repos that historically co-change with this one but aren't in the diff.

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,11 @@ def _as_path(entry: Any) -> str | None:
4444
_CF_VIOLATION_LIMIT = 5
4545
_CF_CYCLE_LIMIT = 3
4646

47+
#: Caps on the will-break split. Production impact leads the directive, so it
48+
#: keeps the larger budget; test fallout is a secondary signal capped tighter.
49+
_WILL_BREAK_LIMIT = 5
50+
_WILL_BREAK_TESTS_LIMIT = 3
51+
4752

4853
def _breaking_change_directive(repo_alias: str) -> list[dict[str, Any]]:
4954
"""Breaking-change half of the PR directive: incompatible provider changes.
@@ -269,6 +274,7 @@ def _build_pr_directive(
269274
exclude_spec: Any,
270275
collector: OmissionCollector,
271276
governance_risk: list[dict[str, Any]],
277+
test_paths: set[str],
272278
alias: str,
273279
) -> None:
274280
"""Assemble PR-mode output: trim co-change lists + blast radius, then build
@@ -295,10 +301,13 @@ def _build_pr_directive(
295301
# entry is a file path (string), never a dossier. Designed to answer
296302
# "what should I do about this PR" in three lines.
297303

298-
will_break = filter_path_list(
304+
affected = filter_path_list(
299305
[p for p in (_as_path(e) for e in trimmed_blast.get("transitive_affected", [])) if p],
300306
exclude_spec,
301-
)[:5]
307+
)
308+
will_break = [p for p in affected if p not in test_paths][:_WILL_BREAK_LIMIT]
309+
will_break_tests = [p for p in affected if p in test_paths][:_WILL_BREAK_TESTS_LIMIT]
310+
302311
missing_cochanges = filter_path_list(
303312
[p for p in (_as_path(e) for e in trimmed_blast.get("cochange_warnings", [])) if p],
304313
exclude_spec,
@@ -364,6 +373,7 @@ def _build_pr_directive(
364373

365374
response["directive"] = {
366375
"will_break": will_break,
376+
"will_break_tests": will_break_tests,
367377
"missing_cochanges": missing_cochanges,
368378
"missing_tests": missing_tests,
369379
"will_break_consumers": will_break_consumers,
@@ -376,6 +386,7 @@ def _build_pr_directive(
376386
"summary": (
377387
f"PR touches {len(changed_files)} file(s). "
378388
f"~{len(will_break)} downstream file(s) likely affected, "
389+
f"{len(will_break_tests)} test(s) likely broken, "
379390
f"{len(missing_cochanges)} historical co-changer(s) missing, "
380391
f"{len(missing_tests)} file(s) without tests."
381392
f"{gov_suffix}{xr_suffix}{bc_suffix}{cf_suffix}"

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ async def get_risk(
8080
select(GraphNode).where(GraphNode.repository_id == repo_id)
8181
)
8282
node_meta = {n.node_id: n for n in node_res.scalars().all()}
83+
test_paths = {nid for nid, n in node_meta.items() if n.is_test}
8384

8485
# Team size is repo-wide — compute once, share across targets
8586
# (small-team calibration for bus-factor-risk, issue #361).
@@ -160,6 +161,7 @@ async def get_risk(
160161
exclude_spec,
161162
collector,
162163
governance_risk,
164+
test_paths,
163165
ctx.alias,
164166
)
165167
else:

tests/unit/server/mcp/conftest.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,20 @@ async def populated_db(session: AsyncSession, repo_id: str) -> str:
325325
community_id=2,
326326
created_at=_NOW,
327327
),
328+
GraphNode(
329+
id="gn4",
330+
repository_id=rid,
331+
node_id="tests/test_service.py",
332+
node_type="file",
333+
language="python",
334+
symbol_count=1,
335+
is_test=True,
336+
is_entry_point=False,
337+
pagerank=0.1,
338+
betweenness=0.0,
339+
community_id=1,
340+
created_at=_NOW,
341+
),
328342
]
329343
for n in nodes:
330344
session.add(n)
@@ -347,6 +361,14 @@ async def populated_db(session: AsyncSession, repo_id: str) -> str:
347361
imported_names_json='["AuthService"]',
348362
created_at=_NOW,
349363
),
364+
GraphEdge(
365+
id="ge3",
366+
repository_id=rid,
367+
source_node_id="tests/test_service.py",
368+
target_node_id="src/auth/service.py",
369+
imported_names_json='["AuthService"]',
370+
created_at=_NOW,
371+
),
350372
]
351373
for e in edges:
352374
session.add(e)

tests/unit/server/mcp/test_risk.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,27 @@ async def test_get_risk_stable_file(setup_mcp):
9191
assert t["risk_type"] == "stable"
9292

9393

94+
@pytest.mark.asyncio
95+
async def test_get_risk_pr_directive_splits_test_breakage(setup_mcp):
96+
"""PR mode splits test-file fallout out of will_break into will_break_tests (#672)."""
97+
from repowise.server.mcp_server import get_risk
98+
99+
# Pass changed_files to trigger PR mode + blast-radius directive.
100+
result = await get_risk(["src/auth/service.py"], changed_files=["src/auth/service.py"])
101+
directive = result["directive"]
102+
103+
# middleware.py imports service.py → production breakage.
104+
assert "src/auth/middleware.py" in directive["will_break"]
105+
assert "src/auth/middleware.py" not in directive["will_break_tests"]
106+
107+
# test_service.py imports service.py but is is_test=True → segmented out.
108+
assert "tests/test_service.py" in directive["will_break_tests"]
109+
assert "tests/test_service.py" not in directive["will_break"]
110+
111+
# Summary reflects the test count.
112+
assert "test(s) likely broken" in directive["summary"]
113+
114+
94115
# ---- _classify_risk_type small-team calibration (issue #361) ---------------
95116

96117

0 commit comments

Comments
 (0)