From b50e1b359085b54ff6bb5cfcf3d88b90d53dfafb Mon Sep 17 00:00:00 2001 From: xiehuanyi <74809554+xiehuanyi@users.noreply.github.com> Date: Thu, 8 Oct 2026 14:07:11 +0300 Subject: [PATCH] fix(python): preserve nested class identities by enclosing class Co-authored-by: Codex (cherry picked from commit e85a748445d286f05601b9bdfaa8feb4602d5f17) --- ARCHITECTURE.md | 5 ++ graphify/cache.py | 2 +- graphify/extract.py | 2 +- graphify/extractors/engine.py | 8 ++- tests/test_python_nested_class_identity.py | 83 ++++++++++++++++++++++ 5 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 tests/test_python_nested_class_identity.py diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 23c899a5a..f69b39712 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -113,6 +113,11 @@ pytest tests/ -q The test suite is designed to avoid network access and uncontrolled filesystem effects; most tests are isolated with temporary directories and environment fixtures. +Python nested class ids include their enclosing class id. Same-named nested +classes under different parents retain separate definitions, methods, and +containment edges; top-level class ids are unchanged. AST cache schema 7 prevents +older cached definitions from retaining the collapsed identities. + R extraction preserves symbol-only bindings such as `%||%` as separate nodes. When normal ID normalization removes an entire binding name, its UTF-8 bytes provide a deterministic operator suffix within the same lexical owner. Its double diff --git a/graphify/cache.py b/graphify/cache.py index 1a7ec68c6..f2530eb42 100644 --- a/graphify/cache.py +++ b/graphify/cache.py @@ -36,7 +36,7 @@ except Exception: _EXTRACTOR_VERSION = "unknown" # Bump when AST cache-key semantics change independently of the package version. -_AST_CACHE_SCHEMA = 7 # Python opaque-base marker and super(args) flag on raw calls. +_AST_CACHE_SCHEMA = 7 # Python opaque-base/super(args) raw-call markers and nested-class enclosing ids. # Version dirs already swept this process — cleanup runs once per (base, version). _cleaned_ast_dirs: set[str] = set() diff --git a/graphify/extract.py b/graphify/extract.py index b6f8f4d06..95ab7d5ea 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -1818,7 +1818,7 @@ def _extract_python_rationale(path: Path, result: dict) -> None: body = node.child_by_field_name("body") if name_node and body: class_name = source[name_node.start_byte:name_node.end_byte].decode("utf-8", errors="replace") - nid = _make_id(stem, class_name) + nid = _make_id(parent_nid, class_name) if parent_nid != file_nid else _make_id(stem, class_name) ds = _get_docstring(body) if ds: _add_rationale(ds[0], ds[1], nid) diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index b81d11c8d..f4c66d0d3 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -4805,6 +4805,8 @@ def _extract_generic( ruby_segments = class_name.split("::") class_name = "::".join(ruby_namespace + ruby_segments) class_nid = _make_id(stem, ".".join(namespace_stack), class_name) + if config.ts_module == "tree_sitter_python" and parent_class_nid: + class_nid = _make_id(parent_class_nid, class_name) line = node.start_point[0] + 1 metadata = None ruby_reopened = ( @@ -4871,8 +4873,8 @@ def _extract_generic( # every language and is always a real class-like node (never a # namespace — namespace handlers pass it through unchanged), so it is # a valid edge source. The `!= class_nid` guard avoids a self-loop - # when same-name nesting (`class Foo: class Foo`) collides ids, since - # class ids omit the enclosing type name. Top-level types (parent + # when same-name nesting (`class Foo: class Foo`) collides ids in + # languages whose class ids omit the enclosing type. Top-level types (parent # None) still source from the file, keeping the containment tree # connected: file -> Outer -> Inner. if parent_class_nid and parent_class_nid != class_nid: @@ -6825,6 +6827,8 @@ def _extract_generic( if inner_name and normalize_id(inner_name): if inner.type in config.class_types: owner_nid = _make_id(stem, ".".join(namespace_stack), inner_name) + if config.ts_module == "tree_sitter_python" and parent_class_nid: + owner_nid = _make_id(parent_class_nid, inner_name) elif parent_class_nid: owner_nid = _make_id(parent_class_nid, inner_name) else: diff --git a/tests/test_python_nested_class_identity.py b/tests/test_python_nested_class_identity.py new file mode 100644 index 000000000..e7c6b46da --- /dev/null +++ b/tests/test_python_nested_class_identity.py @@ -0,0 +1,83 @@ +"""Nested Python class definitions must keep their lexical owner (#4220).""" + +from graphify.extract import extract + + +SOURCE = ( + "class Forward:\n" + " class Params:\n" + " def value(self): return 1\n" + " class Wrapper:\n" + " class Params:\n" + " def value(self): return 2\n" + "class Backward:\n" + " class Params:\n" + " def value(self): return 3\n" +) + + +def _extract(root, source=SOURCE): + root.mkdir(parents=True, exist_ok=True) + path = root / "scheduler.py" + path.write_text(source, encoding="utf-8") + return extract([path], root=root, cache_root=root / "cache", parallel=False) + + +def test_same_named_nested_classes_keep_nodes_methods_and_parent(tmp_path): + result = _extract(tmp_path) + params = [node for node in result["nodes"] if node["label"] == "Params"] + assert len(params) == 3 + assert len({node["id"] for node in params}) == 3 + assert {node["source_location"] for node in params} == {"L2", "L5", "L8"} + owners = {node["id"]: node["label"] for node in result["nodes"]} + expected = {"L2": "Forward", "L5": "Wrapper", "L8": "Backward"} + for node in params: + parents = [edge["source"] for edge in result["edges"] + if edge["relation"] == "contains" and edge["target"] == node["id"]] + assert len(parents) == 1 + assert owners[parents[0]] == expected[node["source_location"]] + methods = [edge["target"] for edge in result["edges"] + if edge["relation"] == "method" and edge["source"] == node["id"]] + assert len(methods) == 1 + assert owners[methods[0]] == ".value()" + assert len([node for node in result["nodes"] if node["label"] == ".value()"]) == 3 + + +def test_same_name_as_parent_has_distinct_identity(tmp_path): + result = _extract(tmp_path, "class Params:\n class Params:\n pass\n") + params = [node for node in result["nodes"] if node["label"] == "Params"] + assert len(params) == 2 + outer, inner = sorted(params, key=lambda node: node["source_location"]) + assert outer["id"] == "scheduler_params" + assert outer["id"] != inner["id"] + assert any(edge["source"] == outer["id"] and edge["target"] == inner["id"] + and edge["relation"] == "contains" for edge in result["edges"]) + + +def test_nested_class_identity_is_stable_across_cache_and_checkout(tmp_path): + first = _extract(tmp_path / "first") + warm = _extract(tmp_path / "first") + relocated = _extract(tmp_path / "second") + assert first["nodes"] == warm["nodes"] == relocated["nodes"] + assert first["edges"] == warm["edges"] == relocated["edges"] + + +def test_nested_class_decorators_and_docstrings_have_real_owners(tmp_path): + result = _extract(tmp_path, ( + "def decorate(cls): return cls\n" + "class Outer:\n" + " @decorate\n" + " class Inner:\n" + " \"\"\"This class holds the nested parameters for the outer class.\"\"\"\n" + " def value(self):\n" + " \"\"\"Return the value owned by this nested class instance.\"\"\"\n" + " return 1\n" + )) + nodes = {node["id"]: node for node in result["nodes"]} + inner = next(node["id"] for node in nodes.values() if node["label"] == "Inner") + method = next(node["id"] for node in nodes.values() if node["label"] == ".value()") + assert any(edge["source"] == inner and edge.get("context") == "decorator" + for edge in result["edges"]) + rationales = [edge for edge in result["edges"] if edge["relation"] == "rationale_for"] + assert {edge["target"] for edge in rationales} == {inner, method} + assert all(edge["source"] in nodes and edge["target"] in nodes for edge in rationales)