Skip to content

Commit e636e35

Browse files
committed
Address Copilot review on #576
- tests: identify rationale nodes by file_type metadata, not id substring (decouples regression from current `<stem>_rationale_<line>` naming) - tests: assert contact_form -> SQLiteQueue.enqueue calls edge directly (previously only inversions were guarded; a dropped enqueue edge would have passed silently) - extract.py: clarify cross-file import indexing comment — the filter intentionally indexes only class-level entities (function/method labels end in "()" and are excluded by design)
1 parent 8a5e06c commit e636e35

2 files changed

Lines changed: 24 additions & 4 deletions

File tree

graphify/extract.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2612,8 +2612,11 @@ def _resolve_cross_file_imports(
26122612
stem = Path(src).stem
26132613
label = node.get("label", "")
26142614
nid = node.get("id", "")
2615-
# Only index real classes/functions (not file nodes, not method stubs,
2616-
# and not rationale nodes whose labels are free-form docstring text — #563)
2615+
# Index class-level entities only. Function/method labels end in "()"
2616+
# so are excluded by the `endswith(")")` filter; file nodes end in ".py";
2617+
# private/internal labels start with "_"; rationale nodes carry
2618+
# file_type=="rationale" and must never participate in cross-file
2619+
# import resolution (#563).
26172620
if (
26182621
label
26192622
and not label.endswith((")", ".py"))

tests/test_issue_563.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,13 +125,25 @@ def repro_graph(tmp_path: Path) -> dict:
125125
# ─────────────────────────────────────────────────────────────────────────────
126126

127127

128+
def _file_type_map(graph: dict) -> dict[str, str]:
129+
"""Map node id -> file_type from the serialized graph.
130+
131+
Identifying rationale nodes via node metadata (file_type == "rationale")
132+
is more durable than substring-matching on the id, which couples to the
133+
current `<stem>_rationale_<line>` naming scheme.
134+
"""
135+
return {n["id"]: n.get("file_type", "") for n in graph["nodes"]}
136+
137+
128138
def test_no_rationale_nodes_in_uses_or_calls_edges(repro_graph):
129139
"""Rationale nodes describe code; they cannot `use`, `call`, or `reference`
130140
anything. The only relation a rationale node may participate in is
131141
`rationale_for` (extractor emits `rationale --rationale_for--> parent`)."""
142+
ftype = _file_type_map(repro_graph)
132143
bad = [
133144
e for e in repro_graph["links"]
134-
if ("rationale" in e["source"] or "rationale" in e["target"])
145+
if (ftype.get(e["source"]) == "rationale"
146+
or ftype.get(e["target"]) == "rationale")
135147
and e["relation"] != "rationale_for"
136148
]
137149
assert bad == [], (
@@ -146,9 +158,10 @@ def test_rationale_for_direction_is_rationale_to_parent(repro_graph):
146158
for this code entity": source is the rationale node, target is the parent.
147159
The export-side direction restore (#563) was the only thing that made
148160
this hold under undirected NetworkX storage."""
161+
ftype = _file_type_map(repro_graph)
149162
rats = [e for e in repro_graph["links"] if e["relation"] == "rationale_for"]
150163
assert rats, "expected some rationale_for edges in the repro"
151-
bad = [e for e in rats if "rationale" not in e["source"]]
164+
bad = [e for e in rats if ftype.get(e["source"]) != "rationale"]
152165
assert bad == [], (
153166
f"rationale_for edges must have a rationale node as source. "
154167
f"Found {len(bad)} flipped: {bad[:3]}"
@@ -177,6 +190,10 @@ def test_calls_direction_is_caller_to_callee(repro_graph):
177190
"contact_form_contact_form",
178191
"jobq_sqlitequeue_check_idempotency",
179192
) in calls, f"expected caller->callee edge missing. got: {calls}"
193+
assert (
194+
"contact_form_contact_form",
195+
"jobq_sqlitequeue_enqueue",
196+
) in calls, f"contact_form->enqueue edge missing. got: {calls}"
180197

181198
# The test function calls contact_form()
182199
assert (

0 commit comments

Comments
 (0)