Skip to content

Commit 415e23b

Browse files
SHudiciclaude
andcommitted
fix: read TESTED_BY edges in the parser's direction everywhere
The parser emits TESTED_BY as production -> test (parser.py, all five language blocks): source is the tested symbol, target is the test. Half the consumers read the edge the other way around (test -> production), so they silently returned nothing on real graphs: - tools/query.py tests_for: queried incoming edges of the production node and returned edge sources. Real graphs never have TESTED_BY edges targeting production code, so the edge-based path always came up empty and only the test_<name> naming fallback ever fired. - graph.py get_transitive_tests (direct, bare-name fallback, and transitive blocks): same inversion, so direct and transitive test coverage was always empty. This also silently zeroed the test coverage factor in compute_risk_score, which calls it. - graph.py load_flow_adjacency: has_tested_by collected edge targets (the tests) instead of sources (the tested code), so flow criticality's tested_count was always 0. - changes.py test-gap detection: checked incoming edges, so every changed function was flagged as a test gap regardless of coverage. - enrich.py: the Tests: context line never listed anything. - refactor.py dead-code detection: has_test_refs looked for incoming TESTED_BY edges and bare-name matches on the target column; both are impossible under the emitted direction. Test references are the node's outgoing TESTED_BY edges (with a bare-source fallback, since the edge source inherits an unresolved CALLS target name). Consumers that already read the emitted direction (risk_index in tools/build.py, knowledge gaps and suggested questions in analysis.py, review context in tools/review.py) are unchanged. Test seeds in test_changes, test_enrich, test_graph and test_integration_v2 seeded the inverted direction to match the inverted consumers; they now seed what the parser emits. New regression tests pin the direction at both ends: the parser must emit production -> test (test_parser), and tests_for must find a test whose name defeats the naming-convention fallback, via the edge alone (test_tools). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent b72413c commit 415e23b

12 files changed

Lines changed: 129 additions & 47 deletions

File tree

code_review_graph/changes.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ def analyze_changes(
352352
for node in changed_funcs:
353353
if node.is_test:
354354
continue
355-
tested = store.get_edges_by_target(node.qualified_name)
355+
tested = store.get_edges_by_source(node.qualified_name)
356356
if not any(e.kind == "TESTED_BY" for e in tested):
357357
test_gaps.append({
358358
"name": _sanitize_name(node.name),

code_review_graph/communities.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -830,8 +830,8 @@ def get_architecture_overview(store: GraphStore) -> dict[str, Any]:
830830
cross_counts: Counter[tuple[int, int]] = Counter()
831831

832832
for e in all_edges:
833-
# TESTED_BY edges are expected cross-community coupling (test → code),
834-
# not an architectural smell.
833+
# TESTED_BY edges are expected cross-community coupling (code and
834+
# its tests), not an architectural smell.
835835
if e.kind == "TESTED_BY":
836836
continue
837837
src_comm = node_to_community.get(e.source_qualified)

code_review_graph/enrich.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,11 +158,11 @@ def _format_node_context(
158158
if flow_names:
159159
lines.append(f" Flows: {', '.join(flow_names)}")
160160

161-
# Tests
161+
# Tests (TESTED_BY edges point production -> test)
162162
tests: list[str] = []
163-
for e in store.get_edges_by_target(qn):
163+
for e in store.get_edges_by_source(qn):
164164
if e.kind == "TESTED_BY" and len(tests) < 3:
165-
t = store.get_node(e.source_qualified)
165+
t = store.get_node(e.target_qualified)
166166
if t:
167167
tests.append(t.name)
168168
if tests:

code_review_graph/graph.py

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ def get_transitive_tests(
377377
) -> list[dict]:
378378
"""Find tests covering a node, including indirect (transitive) coverage.
379379
380-
1. Direct: TESTED_BY edges targeting this node (+ bare-name fallback).
380+
1. Direct: TESTED_BY edges from this node to its tests (+ bare-name fallback).
381381
2. Indirect: follow outgoing CALLS edges up to *max_depth* hops,
382382
then collect TESTED_BY edges on each callee.
383383
@@ -421,31 +421,32 @@ def _node_dict(qn: str, indirect: bool) -> dict | None:
421421
"indirect": indirect,
422422
}
423423

424-
# Direct TESTED_BY
424+
# Direct TESTED_BY (edges point production -> test)
425425
for qn in input_qns:
426426
for row in conn.execute(
427-
"SELECT source_qualified FROM edges "
428-
"WHERE target_qualified = ? AND kind = 'TESTED_BY'",
427+
"SELECT target_qualified FROM edges "
428+
"WHERE source_qualified = ? AND kind = 'TESTED_BY'",
429429
(qn,),
430430
).fetchall():
431-
src = row["source_qualified"]
432-
if src not in seen:
433-
seen.add(src)
434-
d = _node_dict(src, indirect=False)
431+
tst = row["target_qualified"]
432+
if tst not in seen:
433+
seen.add(tst)
434+
d = _node_dict(tst, indirect=False)
435435
if d:
436436
results.append(d)
437437

438-
# Bare-name fallback for direct
438+
# Bare-name fallback for direct: the edge source inherits the CALLS
439+
# target, which can be a bare name when unresolved cross-file.
439440
bare = qualified_name.rsplit("::", 1)[-1] if "::" in qualified_name else qualified_name
440441
for row in conn.execute(
441-
"SELECT source_qualified FROM edges "
442-
"WHERE target_qualified = ? AND kind = 'TESTED_BY'",
442+
"SELECT target_qualified FROM edges "
443+
"WHERE source_qualified = ? AND kind = 'TESTED_BY'",
443444
(bare,),
444445
).fetchall():
445-
src = row["source_qualified"]
446-
if src not in seen:
447-
seen.add(src)
448-
d = _node_dict(src, indirect=False)
446+
tst = row["target_qualified"]
447+
if tst not in seen:
448+
seen.add(tst)
449+
d = _node_dict(tst, indirect=False)
449450
if d:
450451
results.append(d)
451452

@@ -464,14 +465,14 @@ def _node_dict(qn: str, indirect: bool) -> dict | None:
464465
next_frontier = set(list(next_frontier)[:max_frontier])
465466
for callee in next_frontier:
466467
for row in conn.execute(
467-
"SELECT source_qualified FROM edges "
468-
"WHERE target_qualified = ? AND kind = 'TESTED_BY'",
468+
"SELECT target_qualified FROM edges "
469+
"WHERE source_qualified = ? AND kind = 'TESTED_BY'",
469470
(callee,),
470471
).fetchall():
471-
src = row["source_qualified"]
472-
if src not in seen:
473-
seen.add(src)
474-
d = _node_dict(src, indirect=True)
472+
tst = row["target_qualified"]
473+
if tst not in seen:
474+
seen.add(tst)
475+
d = _node_dict(tst, indirect=True)
475476
if d:
476477
results.append(d)
477478
frontier = next_frontier
@@ -1269,8 +1270,8 @@ def load_flow_adjacency(self) -> "FlowAdjacency":
12691270
kind, src, tgt = row["kind"], row["source_qualified"], row["target_qualified"]
12701271
if kind == "CALLS":
12711272
calls_out.setdefault(src, []).append(tgt)
1272-
else: # TESTED_BY
1273-
has_tested_by.add(tgt)
1273+
else: # TESTED_BY points production -> test; the SOURCE is tested
1274+
has_tested_by.add(src)
12741275

12751276
return FlowAdjacency(
12761277
calls_out=calls_out,

code_review_graph/refactor.py

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -492,19 +492,29 @@ def _is_plausible_caller(
492492
if _is_plausible_caller(e.file_path, node.file_path, node.name)
493493
]
494494
incoming = incoming + all_bare
495-
if not any(e.kind == "TESTED_BY" for e in incoming):
496-
bare_tb = store.search_edges_by_target_name(node.name, kind="TESTED_BY")
497-
bare_tb = [
498-
e for e in bare_tb
495+
# TESTED_BY edges point production -> test, so a node's test
496+
# references are its OUTGOING edges. The edge source inherits the
497+
# CALLS target, which can be a bare name when unresolved cross-file.
498+
test_refs = [
499+
e for e in store.get_edges_by_source(node.qualified_name)
500+
if e.kind == "TESTED_BY"
501+
]
502+
if not test_refs:
503+
bare_tb_rows = conn.execute(
504+
"SELECT * FROM edges WHERE kind = 'TESTED_BY'"
505+
" AND source_qualified = ?",
506+
(node.name,),
507+
).fetchall()
508+
test_refs = [
509+
e for e in (store._row_to_edge(r) for r in bare_tb_rows)
499510
if _is_plausible_caller(e.file_path, node.file_path, node.name)
500511
]
501-
incoming = incoming + bare_tb
502512
# Check INHERITS -- classes with subclasses are not dead.
503513
if node.kind == "Class" and not any(e.kind == "INHERITS" for e in incoming):
504514
bare_inh = store.search_edges_by_target_name(node.name, kind="INHERITS")
505515
incoming = incoming + bare_inh
506516
has_callers = any(e.kind == "CALLS" for e in incoming)
507-
has_test_refs = any(e.kind == "TESTED_BY" for e in incoming)
517+
has_test_refs = bool(test_refs)
508518
has_importers = any(e.kind == "IMPORTS_FROM" for e in incoming)
509519
has_references = any(e.kind == "REFERENCES" for e in incoming)
510520
has_subclasses = any(e.kind == "INHERITS" for e in incoming)

code_review_graph/tools/query.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -293,9 +293,10 @@ def query_graph(
293293
results.append(node_to_dict(child))
294294

295295
elif pattern == "tests_for":
296-
for e in store.get_edges_by_target(qn):
296+
# TESTED_BY edges point production -> test.
297+
for e in store.get_edges_by_source(qn):
297298
if e.kind == "TESTED_BY":
298-
test = store.get_node(e.source_qualified)
299+
test = store.get_node(e.target_qualified)
299300
if test:
300301
results.append(node_to_dict(test))
301302
# Also search by naming convention

tests/test_changes.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,11 @@ def _add_call(self, source_qn: str, target_qn: str, path: str = "app.py") -> Non
6464
self.store.commit()
6565

6666
def _add_tested_by(self, test_qn: str, target_qn: str, path: str = "app.py") -> None:
67+
# The parser emits TESTED_BY as production (target_qn) -> test (test_qn).
6768
edge = EdgeInfo(
6869
kind="TESTED_BY",
69-
source=test_qn,
70-
target=target_qn,
70+
source=target_qn,
71+
target=test_qn,
7172
file_path=path,
7273
line=1,
7374
)

tests/test_enrich.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@ def _seed_data(self):
100100
),
101101
EdgeInfo(
102102
kind="TESTED_BY",
103-
source=f"{self.tmpdir}/test_parser.py::test_parse_file",
104-
target=f"{self.tmpdir}/parser.py::parse_file",
103+
source=f"{self.tmpdir}/parser.py::parse_file",
104+
target=f"{self.tmpdir}/test_parser.py::test_parse_file",
105105
file_path=f"{self.tmpdir}/test_parser.py", line=1,
106106
),
107107
]

tests/test_graph.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ def test_uncapped_small_frontier_unchanged(self):
401401
# Only callee_2 has a test
402402
if i == 2:
403403
self.store.upsert_edge(EdgeInfo(
404-
kind="TESTED_BY", source=test_qn, target=callee_qn,
404+
kind="TESTED_BY", source=callee_qn, target=test_qn,
405405
file_path="/t/test_hub.py", line=1,
406406
))
407407
self.store.commit()

tests/test_integration_v2.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,8 @@ def _seed_realistic_graph(self):
180180

181181
# --- Edges: tested_by ---
182182
s.upsert_edge(EdgeInfo(
183-
kind="TESTED_BY", source="test_auth.py::test_login",
184-
target="auth.py::login", file_path="test_auth.py", line=5,
183+
kind="TESTED_BY", source="auth.py::login",
184+
target="test_auth.py::test_login", file_path="test_auth.py", line=5,
185185
))
186186

187187
s.commit()

0 commit comments

Comments
 (0)