From 23bbe7e13c4430e4d8fd4e7db509184df4dde8ba Mon Sep 17 00:00:00 2001 From: phudayyy Date: Thu, 13 Aug 2026 12:25:40 +0700 Subject: [PATCH] fix: `affected` silently returns nothing for equivalent path forms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resolve_seed` compares the query to the stored `source_file` as a plain string, so a file can be named three ways and only one of them resolves: graphify affected src/x.py -> 16 results, exit 0 graphify affected ./src/x.py -> 0 results, exit 0 graphify affected /abs/src/x.py -> 0 results, exit 0 graphify affected typo.py -> 0 results, exit 0 The last two lines are the problem. A blast-radius tool answering "nothing depends on this" is an answer people act on, and here it is indistinguishable both from a genuine zero and from a typo — same empty list, same exit 0, no warning. `./` is what shell completion produces and an absolute path is what any script passes. Found while measuring `affected` against an independently built import graph: the probe passed absolute paths and measured recall 0.000 for every module, which is the same failure one layer up. Fix: normalise the query to repo-relative form for the `source_file` comparison only. The label branches above keep the query verbatim, and non-path queries are unaffected -- `Path("myFunc()").as_posix()` is `"myFunc()"`. An absolute path rooted outside the repo is left alone rather than guessed at by basename, which would match an unrelated file of the same name. Test: `test_affected_resolves_equivalent_path_forms` is red without the change and green with it. Suite: 5 failed / 4252 passed without the patch, 5 failed / 4253 passed with it -- identical failures, all in `tests/test_ollama_retry_cap.py` (no ollama in this environment), plus the one new test. --- graphify/affected.py | 34 ++++++++++++++++++++++++++++++++-- tests/test_affected_cli.py | 26 ++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/graphify/affected.py b/graphify/affected.py index 0a3cd157b..543772f12 100644 --- a/graphify/affected.py +++ b/graphify/affected.py @@ -67,6 +67,31 @@ def _normalize_label(label: str) -> str: return unicodedata.normalize("NFC", label).casefold() +def _as_repo_relative(query: str) -> str: + """Repo-relative form of a path query, for matching a stored `source_file`. + + The graph stores repo-relative paths, so `./src/x.py` and + `/abs/repo/src/x.py` name the same file as `src/x.py` and yet matched + nothing. `affected` then printed an empty list and exited 0 — a blast-radius + tool answering "nothing depends on this" about a file with sixteen + dependents, and indistinguishable from a genuine zero or a typo. + + Non-path queries pass through unchanged: `Path("myFunc()").as_posix()` is + `"myFunc()"`, so label resolution is untouched. An absolute path rooted + outside the repo is left alone — no basename guessing. + """ + path = Path(query) + if path.is_absolute(): + try: + return path.relative_to(Path.cwd()).as_posix() + except ValueError: + # Rooted outside the repo: nothing here can make it repo-relative, + # so leave it alone rather than guess at a basename that would match + # some unrelated file with the same name. + return query + return path.as_posix() + + def _prefer_file_node( graph: nx.Graph, node_ids: list[str], @@ -128,15 +153,20 @@ def resolve_seed(graph: nx.Graph, query: str) -> str | None: ] if len(bare_name_matches) == 1: return bare_name_matches[0] + # Compare paths in repo-relative form. Only this branch is path-shaped; the + # label branches above keep the query verbatim. + query_path = _normalize_label(_as_repo_relative(query)) exact_source_matches = [ str(node_id) for node_id, data in graph.nodes(data=True) - if _normalize_label(str(data.get("source_file", ""))) == query_lower + if _normalize_label(str(data.get("source_file", ""))) in (query_lower, query_path) ] if len(exact_source_matches) == 1: return exact_source_matches[0] if exact_source_matches: - preferred_file_node = _prefer_file_node(graph, exact_source_matches, query) + preferred_file_node = _prefer_file_node( + graph, exact_source_matches, _as_repo_relative(query) + ) if preferred_file_node is not None: return preferred_file_node contains_matches = [ diff --git a/tests/test_affected_cli.py b/tests/test_affected_cli.py index ca608b6b3..05798e48b 100644 --- a/tests/test_affected_cli.py +++ b/tests/test_affected_cli.py @@ -311,3 +311,29 @@ def test_affected_falls_back_to_def_line_when_edge_has_no_location(monkeypatch, monkeypatch.setattr(mainmod.sys, "argv", ["graphify", "affected", "target", "--graph", str(gp)]) mainmod.main() assert "a.py:L90" in capsys.readouterr().out + + +def test_affected_resolves_equivalent_path_forms(tmp_path, monkeypatch): + """`./x.py`, an absolute path and `x.py` name one file and must resolve alike. + + The graph stores repo-relative `source_file`, and `resolve_seed` compared the + query to it as a plain string. `./pkg/foo.py` and `/abs/repo/pkg/foo.py` + therefore matched nothing, `affected` printed an empty list and exited 0 — a + blast-radius tool reporting "nothing depends on this" about a file with three + dependents, and indistinguishable both from a genuine zero and from a typo. + """ + from graphify.affected import resolve_seed + + graph = nx.DiGraph() + graph.add_node("target", label="Foo", source_file="pkg/foo.py", source_location="L1") + graph.add_node("caller", label="X()", source_file="app.py", source_location="L4") + graph.add_edge("caller", "target", relation="calls") + + monkeypatch.chdir(tmp_path) + for query in ( + "pkg/foo.py", + "./pkg/foo.py", + str(tmp_path / "pkg" / "foo.py"), + ): + assert resolve_seed(graph, query) == "target", query +