mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-28 09:17:02 +00:00
fix(affected): resolve an absolute-path seed via the graph-derived root (#2706)
affected anchored an absolute-path seed to Path.cwd(), so running it from anywhere but the repo root made relative_to(cwd) raise and the query fell through unmatched, silently returning nothing. It now derives the repo root from the graph's own location (<root>/graphify-out/graph.json) and anchors the seed there, so an absolute seed resolves regardless of cwd. Composes with the #2707 relative-seed fix (root defaults to cwd for other callers). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
safishamsi
co-authored by
Claude Opus 4.8
parent
e5d662eb63
commit
243a1801f1
+16
-7
@@ -67,7 +67,7 @@ def _normalize_label(label: str) -> str:
|
||||
return unicodedata.normalize("NFC", label).casefold()
|
||||
|
||||
|
||||
def _as_repo_relative(query: str) -> str:
|
||||
def _as_repo_relative(query: str, root: Path | None = None) -> 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
|
||||
@@ -76,14 +76,22 @@ def _as_repo_relative(query: str) -> str:
|
||||
tool answering "nothing depends on this" about a file with sixteen
|
||||
dependents, and indistinguishable from a genuine zero or a typo.
|
||||
|
||||
An absolute path is anchored to `root` when given — the repo root derived
|
||||
from the graph's own location — so a seed resolves regardless of the caller's
|
||||
working directory (#2706: an absolute-path seed previously only matched when
|
||||
cwd happened to be the analysed repo root, which no editor or script can
|
||||
guarantee). `root` falls back to the current directory to preserve the prior
|
||||
behaviour when a caller has no graph location to derive it from.
|
||||
|
||||
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.
|
||||
outside `root` is left alone — no basename guessing.
|
||||
"""
|
||||
path = Path(query)
|
||||
if path.is_absolute():
|
||||
anchor = root if root is not None else Path.cwd()
|
||||
try:
|
||||
return path.relative_to(Path.cwd()).as_posix()
|
||||
return path.relative_to(anchor).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
|
||||
@@ -127,7 +135,7 @@ def _prefer_file_node(
|
||||
return None
|
||||
|
||||
|
||||
def resolve_seed(graph: nx.Graph, query: str) -> str | None:
|
||||
def resolve_seed(graph: nx.Graph, query: str, root: Path | None = None) -> str | None:
|
||||
# A trailing path separator must not change a source-file match — serve's
|
||||
# _find_node tokenizes the path (which drops it), so strip it here for parity
|
||||
# (otherwise `affected "src/x.ts/"` returned None while `explain` resolved it).
|
||||
@@ -155,7 +163,7 @@ def resolve_seed(graph: nx.Graph, query: str) -> str | None:
|
||||
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))
|
||||
query_path = _normalize_label(_as_repo_relative(query, root))
|
||||
exact_source_matches = [
|
||||
str(node_id)
|
||||
for node_id, data in graph.nodes(data=True)
|
||||
@@ -165,7 +173,7 @@ def resolve_seed(graph: nx.Graph, query: str) -> str | None:
|
||||
return exact_source_matches[0]
|
||||
if exact_source_matches:
|
||||
preferred_file_node = _prefer_file_node(
|
||||
graph, exact_source_matches, _as_repo_relative(query)
|
||||
graph, exact_source_matches, _as_repo_relative(query, root)
|
||||
)
|
||||
if preferred_file_node is not None:
|
||||
return preferred_file_node
|
||||
@@ -253,9 +261,10 @@ def format_affected(
|
||||
*,
|
||||
relations: Iterable[str] = DEFAULT_AFFECTED_RELATIONS,
|
||||
depth: int = 2,
|
||||
root: Path | None = None,
|
||||
) -> str:
|
||||
relation_list = tuple(relations)
|
||||
seed = resolve_seed(graph, query)
|
||||
seed = resolve_seed(graph, query, root)
|
||||
if seed is None:
|
||||
return f"No unique node match for {query}"
|
||||
|
||||
|
||||
@@ -1112,12 +1112,20 @@ def dispatch_command(cmd: str) -> None:
|
||||
except Exception as exc:
|
||||
print(f"error: could not load graph: {exc}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
# Derive the analysed repo root from the graph's own location so an
|
||||
# absolute-path seed resolves without requiring cwd to be that root
|
||||
# (#2706). The graph is written to <root>/<GRAPHIFY_OUT_NAME>/graph.json,
|
||||
# so the root is the output dir's parent; a graph pointed at directly by
|
||||
# --graph falls back to its own directory.
|
||||
from graphify.paths import GRAPHIFY_OUT_NAME
|
||||
graph_root = gp.parent.parent if gp.parent.name == GRAPHIFY_OUT_NAME else gp.parent
|
||||
print(
|
||||
format_affected(
|
||||
graph,
|
||||
query,
|
||||
relations=relations or DEFAULT_AFFECTED_RELATIONS,
|
||||
depth=depth,
|
||||
root=graph_root,
|
||||
)
|
||||
)
|
||||
elif cmd in ("god-nodes", "god_nodes"):
|
||||
|
||||
@@ -337,3 +337,37 @@ def test_affected_resolves_equivalent_path_forms(tmp_path, monkeypatch):
|
||||
):
|
||||
assert resolve_seed(graph, query) == "target", query
|
||||
|
||||
|
||||
def test_affected_absolute_seed_resolves_via_graph_root_off_cwd(tmp_path, monkeypatch, capsys):
|
||||
"""An absolute-path seed resolves off the graph's location, not the cwd (#2706).
|
||||
|
||||
The shipped `./`/absolute fix only matched when the working directory already
|
||||
was the analysed repo root. Editors and scripts pass an absolute path from
|
||||
anywhere, so `affected` kept answering "nothing depends on this" — the
|
||||
maintainer's noted follow-up. The root is now derived from the graph's own
|
||||
location (`<root>/graphify-out/graph.json`).
|
||||
"""
|
||||
from graphify.paths import GRAPHIFY_OUT_NAME
|
||||
|
||||
repo_root = tmp_path / "repo"
|
||||
out_dir = repo_root / GRAPHIFY_OUT_NAME
|
||||
out_dir.mkdir(parents=True)
|
||||
g = nx.DiGraph()
|
||||
g.add_node("target", label="Foo", source_file="pkg/foo.py", source_location="L1")
|
||||
g.add_node("caller", label="X()", source_file="app.py", source_location="L4")
|
||||
g.add_edge("caller", "target", relation="calls")
|
||||
gp = out_dir / "graph.json"
|
||||
gp.write_text(json.dumps(json_graph.node_link_data(g, edges="links")), encoding="utf-8")
|
||||
|
||||
elsewhere = tmp_path / "elsewhere"
|
||||
elsewhere.mkdir()
|
||||
monkeypatch.chdir(elsewhere) # NOT the repo root — mimics an editor/script caller
|
||||
abs_seed = str(repo_root / "pkg" / "foo.py")
|
||||
monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None)
|
||||
monkeypatch.setattr(mainmod.sys, "argv", ["graphify", "affected", abs_seed, "--graph", str(gp)])
|
||||
mainmod.main()
|
||||
|
||||
out = capsys.readouterr().out
|
||||
assert "Affected nodes for Foo" in out
|
||||
assert "X()" in out
|
||||
|
||||
|
||||
Reference in New Issue
Block a user