diff --git a/graphify/extract.py b/graphify/extract.py index 1d396a69..5caaaa3a 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -85,6 +85,16 @@ def _file_stem(path: Path) -> str: return path.stem +def _file_node_id(rel_path: Path) -> str: + """File-level node ID matching the skill.md spec: ``{parent_dir}_{stem}`` — + one parent directory level, no extension. ``rel_path`` MUST be relative to + the project root so top-level files collapse to a bare stem (``setup.py`` -> + ``setup``) instead of picking up the root directory name. This must equal the + ID semantic subagents generate, or AST and semantic extraction split a file + into two disconnected ghost nodes (#1033).""" + return _make_id(_file_stem(rel_path)) + + _TSCONFIG_ALIAS_CACHE: dict[str, dict[str, str]] = {} _WORKSPACE_PACKAGE_CACHE: dict[str, dict[str, Path]] = {} _JS_CACHE_BYPASS_SUFFIXES = {".js", ".jsx", ".mjs", ".ts", ".tsx", ".vue", ".svelte"} @@ -10370,15 +10380,22 @@ def extract( _augment_symbol_resolution_edges(paths, all_nodes, all_edges, root) - # Remap file node IDs from absolute-path-derived to project-relative so - # graph.json edge endpoints are stable across machines (#502) + # Remap file node IDs from absolute-path-derived to the canonical + # {parent_dir}_{stem} spec form so (a) graph.json edge endpoints are stable + # across machines (#502) and (b) AST file nodes match the IDs semantic + # subagents generate (#1033). Resolve before relativizing so paths passed in + # relative form still anchor to the (resolved) root. id_remap: dict[str, str] = {} for path in paths: old_id = _make_id(str(path)) try: - new_id = _make_id(str(path.relative_to(root))) + rel = path.relative_to(root) except ValueError: - continue + try: + rel = path.resolve().relative_to(root) + except ValueError: + continue + new_id = _file_node_id(rel) if old_id != new_id: id_remap[old_id] = new_id if id_remap: @@ -10462,7 +10479,7 @@ def extract( sf_rel = sf_path.relative_to(root) if sf_path.is_absolute() else sf_path except ValueError: sf_rel = sf_path - nid_to_file_nid[n["id"]] = _make_id(str(sf_rel)) + nid_to_file_nid[n["id"]] = _file_node_id(sf_rel) existing_pairs = {(e["source"], e["target"]) for e in all_edges} for rc in all_raw_calls: diff --git a/graphify/symbol_resolution.py b/graphify/symbol_resolution.py index 7bc68093..39cc60f0 100644 --- a/graphify/symbol_resolution.py +++ b/graphify/symbol_resolution.py @@ -365,14 +365,24 @@ def _bash_make_id(*parts: str) -> str: return cleaned.strip("_").casefold() +def _bash_file_stem(rel_path: Path) -> str: + """Exact copy of extract._file_stem — kept here to avoid an import cycle.""" + parent = rel_path.parent.name + if parent and parent not in (".", ""): + return f"{parent}.{rel_path.stem}" + return rel_path.stem + + def _file_node_id_for_path(path: Path, root: Path) -> str: - # Resolve both sides so callers that pass relative or non-canonical roots - # get the same canonical relative path that extract()'s id_remap produces. - # _bash_make_id is an exact copy of extract._make_id, so IDs match. + # Produce the canonical {parent_dir}_{stem} file-node ID that extract()'s + # id_remap generates (#1033), so bash `source` edges land on the real file + # node instead of an orphan. _bash_make_id / _bash_file_stem are exact copies + # of extract._make_id / extract._file_stem, so IDs match. try: - return _bash_make_id(str(path.resolve().relative_to(root.resolve()))) + rel = path.resolve().relative_to(root.resolve()) except ValueError: return _bash_make_id(str(path)) # path outside root: hash absolute path as fallback + return _bash_make_id(_bash_file_stem(rel)) def resolve_bash_source_edges( diff --git a/tests/test_file_node_id_spec.py b/tests/test_file_node_id_spec.py new file mode 100644 index 00000000..42505667 --- /dev/null +++ b/tests/test_file_node_id_spec.py @@ -0,0 +1,112 @@ +"""Regression tests for issue #1033: AST file-level node IDs must match the +skill.md `{parent_dir}_{stem}` spec (one parent level, no extension) so AST and +semantic extraction produce the SAME node for a file instead of two disconnected +ghosts. + +skill.md spec (line ~390): + stem = {parent_dir}_{filename_without_ext}, lowercased, non-alphanumeric -> _ + examples: + src/auth/session.py + ValidateToken -> auth_session_validatetoken + match/script/pipeline_step.py (file node) -> script_pipeline_step + setup.py (top-level) -> setup +""" +from pathlib import Path + +from graphify.extract import extract + + +def _file_nodes(extraction: dict) -> list[dict]: + # File-level nodes carry a label equal to the file's basename. + return [ + n for n in extraction["nodes"] + if n.get("source_file", "").endswith(n.get("label", "\0")) + and n.get("file_type") == "code" + ] + + +def test_file_node_id_uses_parent_dir_and_stem_no_extension(tmp_path): + """match/script/pipeline_step.py -> file node id 'script_pipeline_step'.""" + sub = tmp_path / "match" / "script" + sub.mkdir(parents=True) + f = sub / "pipeline_step.py" + f.write_text("def run():\n pass\n") + + extraction = extract([f], cache_root=tmp_path) + ids = {n["id"] for n in extraction["nodes"]} + + assert "script_pipeline_step" in ids, ( + f"expected spec-format file id 'script_pipeline_step', got {sorted(ids)}" + ) + # The old buggy full-path-with-extension id must be gone. + assert "match_script_pipeline_step_py" not in ids + assert not any(i.endswith("_py") for i in ids if "pipeline_step" in i) + + +def test_top_level_file_node_id_is_bare_stem(tmp_path): + """A file directly at the project root collapses to just its stem.""" + f = tmp_path / "setup.py" + f.write_text("def configure():\n pass\n") + + extraction = extract([f], cache_root=tmp_path) + ids = {n["id"] for n in extraction["nodes"]} + + assert "setup" in ids, f"expected bare stem 'setup', got {sorted(ids)}" + assert "setup_py" not in ids + + +def test_symbol_and_file_ids_share_the_same_stem(tmp_path): + """Symbol ids already use {parent}_{stem}_{name}; the file node must share + that stem prefix so 'contains' edges connect file -> symbol.""" + sub = tmp_path / "match" / "script" + sub.mkdir(parents=True) + f = sub / "pipeline_step.py" + f.write_text("def run():\n pass\n\nclass Stage:\n pass\n") + + extraction = extract([f], cache_root=tmp_path) + ids = {n["id"] for n in extraction["nodes"]} + + assert "script_pipeline_step" in ids # file node + assert "script_pipeline_step_stage" in ids # class symbol shares stem + + # The file -> class 'contains' edge must reference the real file node id. + contains = [ + e for e in extraction["edges"] + if e["relation"] == "contains" and e["target"] == "script_pipeline_step_stage" + ] + assert contains, "no 'contains' edge to the class symbol" + assert contains[0]["source"] == "script_pipeline_step", ( + f"contains edge source {contains[0]['source']!r} does not match file node" + ) + + +def test_cross_file_import_edges_stay_connected(tmp_path): + """Changing the file-id format must not orphan import edges: the import + target must resolve to the imported file's (new-format) node id.""" + pkg = tmp_path / "pkg" + pkg.mkdir() + (pkg / "models.py").write_text("class User:\n pass\n") + (pkg / "auth.py").write_text( + "from models import User\n\n" + "class Session:\n" + " def check(self):\n" + " return User()\n" + ) + + files = [pkg / "models.py", pkg / "auth.py"] + extraction = extract(files, cache_root=tmp_path) + ids = {n["id"] for n in extraction["nodes"]} + + assert "pkg_models" in ids + assert "pkg_auth" in ids + + # Every edge endpoint that looks like a file node must point at a real node + # (no dangling '*_py' ghosts left behind by the old format). + node_ids = ids + for e in extraction["edges"]: + for endpoint in (e["source"], e["target"]): + assert not endpoint.endswith("_py"), ( + f"edge endpoint {endpoint!r} kept the old extension-suffixed format" + ) + # imports_from edges between files must land on a known node. + if e["relation"] == "imports_from" and e["source"] == "pkg_auth": + assert e["target"] in node_ids or "models" in e["target"] diff --git a/tests/test_js_import_resolution.py b/tests/test_js_import_resolution.py index 414cf8d8..e2efbcee 100644 --- a/tests/test_js_import_resolution.py +++ b/tests/test_js_import_resolution.py @@ -3,7 +3,7 @@ from __future__ import annotations import json from pathlib import Path -from graphify.extract import _file_stem, _make_id, extract +from graphify.extract import _file_node_id, _file_stem, _make_id, extract def _write(path: Path, text: str) -> Path: @@ -17,7 +17,7 @@ def _extract_for(paths: list[Path], root: Path): def _has_edge(result: dict, source: str, target: str, relation: str = "imports_from") -> bool: - expected = (_make_id(source), _make_id(target), relation) + expected = (_file_node_id(Path(source)), _file_node_id(Path(target)), relation) actual = { (edge["source"], edge["target"], edge["relation"]) for edge in result["edges"] @@ -32,7 +32,7 @@ def _has_symbol_edge( symbol: str, relation: str = "imports", ) -> bool: - expected = (_make_id(source), _make_id(_file_stem(Path(target_file)), symbol), relation) + expected = (_file_node_id(Path(source)), _make_id(_file_stem(Path(target_file)), symbol), relation) actual = { (edge["source"], edge["target"], edge["relation"]) for edge in result["edges"]