Fix Python leading-underscore method/function id collision

ids.py:make_id strips leading/trailing underscores from every name
part before normalizing, so _get_connection/get_connection (and more
broadly any name pair differing only by underscore wrapping --
x/_x/__x/__x__ dunder variants all collapse the same way) mint the
SAME node id. add_node's first-wins dedup then silently drops
whichever declaration is walked second: a public method could be
entirely absent from the graph while its private-by-convention
sibling occupied the public name, with no warning and exit 0.

Adds a Python-scoped pre-scan (_python_pre_scan_underscore_collisions)
that groups module-level functions and direct class methods by their
would-be node id, plus a resolver (_python_underscore_salted_nid) that
salts every group member except a unique public (no leading
underscore) one -- mirroring the established convention the Go
extractor already uses for its own case-only collision (#2779/#2780):
when no unique public member exists, every member is salted, so the
outcome never depends on declaration order. The public member's id
stays exactly what it would be with no colliding sibling at all, so
adding/removing a private-by-convention sibling in an incremental
rebuild re-points nothing.

Both hooks are gated on config.ts_module == "tree_sitter_python" and
live inside the shared _extract_generic engine (Python has no
dedicated extractor file, unlike Go/Rust/SQL), following the existing
precedent for per-language pre-scans already there (csharp_interface_names,
swift_protocol_names). Call-site resolution needed no changes: label_to_nid
is rebuilt from the final nodes list after the walk completes, so it
naturally picks up whichever id (salted or not) a method actually got.

9 new tests in tests/test_python_underscore_resolution.py: the exact
repro, id stability across incremental add/remove of a private
sibling, the call edge correctly binding to the salted target, the
module-level (non-method) case, the no-unique-public-member case
(both members salted), a dunder+plain collision, an unaffected
no-collision file, and scope isolation (a collision in one class must
not touch an unrelated same-named-but-non-colliding member in another
class).

Note: tests/ on the current v8 HEAD has 7 pre-existing failures
unrelated to this change (test_cli_export.py, test_cross_repo_shared_types.py,
test_extract_code_only_cli.py, test_merge_graphs_cli.py) -- confirmed via
`git stash` that they fail identically on pristine v8 without this diff.
Full suite otherwise: 5116 passed. ruff and skillgen --check clean.

Fixes #3302.

(cherry picked from commit cb8ccebab9)

graphify/extractors/engine.py

@@ -814,6 +814,84 @@ def _swift_declaration_keyword(node) -> str | None:
             return c.type
     return None
 
+def _python_pre_scan_underscore_collisions(root_node, source: bytes, stem: str) -> dict[str, set[str]]:
+    """Pre-scan a Python module for name-only differences that collapse to one node id.
+
+    ``ids.py:make_id`` strips leading/trailing underscores from every part before
+    normalizing, so ``_get_connection`` and ``get_connection`` mint the SAME id.
+    ``add_node`` then silently drops whichever declaration is walked second — no
+    warning, exit 0 — so a public method can be entirely absent from the graph
+    while its private-by-convention sibling (or a dunder, which strips the same
+    way: ``__x``/``__x__``/``x`` all collapse too) occupies the public name (#3302).
+
+    Returns ``{plain_nid: {raw names that would collide on it}}`` for every
+    module-level function and every direct method of a module-level class — the
+    two cases the id-minting code actually distinguishes via ``parent_class_nid``.
+    Deliberately does not recurse into nested classes or nested functions: this
+    keeps the scope key trivially exact (matching ``_make_id(stem, class_name)``,
+    which only holds because Python never populates ``namespace_stack``) rather
+    than replicating the corpus-wide id-computation rules for every nesting shape.
+    A collision entirely inside an unhandled nested scope is simply not caught
+    here — a strict miss, never a false positive, since the map is only ever
+    consulted for a nid this same scan actually computed.
+    """
+    groups: dict[str, set[str]] = {}
+
+    def _record(plain_nid: str, name: str) -> None:
+        groups.setdefault(plain_nid, set()).add(name)
+
+    for child in root_node.children:
+        if child.type == "function_definition":
+            name_node = child.child_by_field_name("name")
+            if name_node is not None:
+                name = _read_text(name_node, source)
+                if name:
+                    _record(_make_id(stem, name), name)
+        elif child.type == "class_definition":
+            class_name_node = child.child_by_field_name("name")
+            body = child.child_by_field_name("body")
+            if class_name_node is None or body is None:
+                continue
+            class_name = _read_text(class_name_node, source)
+            if not class_name:
+                continue
+            class_nid = _make_id(stem, class_name)
+            for member in body.children:
+                if member.type != "function_definition":
+                    continue
+                name_node = member.child_by_field_name("name")
+                if name_node is None:
+                    continue
+                name = _read_text(name_node, source)
+                if name:
+                    _record(_make_id(class_nid, name), name)
+
+    return {nid: names for nid, names in groups.items() if len(names) >= 2}
+
+
+def _python_underscore_salted_nid(plain_nid: str, name: str, groups: dict[str, set[str]]) -> str:
+    """Resolve a Python function/method's real node id against the collision map.
+
+    A name with no leading underscore at all is "public" per PEP 8 convention.
+    When a collision group has exactly one public member, that member keeps the
+    plain id — cross-file/documentation references overwhelmingly target the
+    public name, and keeping it stable means an incremental rebuild that adds or
+    removes a private-by-convention sibling re-points nothing. Every other
+    member of the group (including the public one when it is NOT unique, e.g.
+    `_x`/`__x` colliding with no public member at all) is salted, so the outcome
+    never depends on declaration order — mirrors the exported/unexported rule
+    the Go extractor uses for its own case-only collision (#2779).
+    """
+    names = groups.get(plain_nid)
+    if not names or len(names) < 2:
+        return plain_nid
+    public = [n for n in names if not n.startswith("_")]
+    if len(public) == 1 and name == public[0]:
+        return plain_nid
+    salt = hashlib.sha1(name.encode("utf-8"), usedforsecurity=False).hexdigest()[:6]
+    return _make_id(plain_nid, salt)
+
+
 def _swift_pre_scan(root_node, source: bytes) -> tuple[set[str], set[str]]:
     """Pre-scan a Swift compilation unit and return (protocol_names, class_like_names)."""
     protocols: set[str] = set()
@@ -3002,6 +3080,10 @@ def _extract_generic(
     if config.ts_module == "tree_sitter_swift":
         swift_protocol_names, swift_class_names = _swift_pre_scan(root, source)
 
+    python_underscore_groups: dict[str, set[str]] = {}
+    if config.ts_module == "tree_sitter_python":
+        python_underscore_groups = _python_pre_scan_underscore_collisions(root, source, stem)
+
     def add_node(nid: str, label: str, line: int, *, node_type: str | None = None,
                  metadata: dict | None = None) -> None:
         if nid in seen_ids:
@@ -4230,10 +4312,18 @@ def _extract_generic(
             line = node.start_point[0] + 1
             if parent_class_nid:
                 func_nid = _make_id(parent_class_nid, sanitized_name)
+                if config.ts_module == "tree_sitter_python":
+                    func_nid = _python_underscore_salted_nid(
+                        func_nid, sanitized_name, python_underscore_groups
+                    )
                 add_node(func_nid, f".{func_name}()", line)
                 add_edge(parent_class_nid, func_nid, "method", line)
             else:
                 func_nid = _make_id(stem, sanitized_name)
+                if config.ts_module == "tree_sitter_python":
+                    func_nid = _python_underscore_salted_nid(
+                        func_nid, sanitized_name, python_underscore_groups
+                    )
                 add_node(func_nid, f"{func_name}()", line)
                 add_edge(file_nid, func_nid, "contains", line)
             callable_def_nids.add(func_nid)  # function / method def is callable

tests/test_python_underscore_resolution.py

@@ -0,0 +1,220 @@
+"""Regression coverage for Python leading-underscore node-id collisions (#3302).
+
+``ids.py:make_id`` strips leading/trailing underscores from every part before
+normalizing, so ``_get_connection``/``get_connection`` (and more broadly any
+name differing only by ``_``/``__``/dunder wrapping, e.g. ``x``/``_x``/``__x``/
+``__x__``) mint the SAME node id. ``add_node`` then silently drops whichever
+declaration is walked second, so a public method/function can be entirely
+absent from the graph while its private-by-convention sibling occupies the
+public name.
+"""
+from __future__ import annotations
+import textwrap
+from pathlib import Path
+
+from graphify.extract import extract_python
+
+
+def _write_py(tmp_path: Path, code: str) -> Path:
+    p = tmp_path / "mod.py"
+    p.write_text(textwrap.dedent(code))
+    return p
+
+
+def _rationale_free_nodes(result: dict) -> list[dict]:
+    return [n for n in result["nodes"] if n.get("file_type") != "rationale"]
+
+
+def test_leading_underscore_method_collision_both_extracted(tmp_path: Path) -> None:
+    """#3302's exact repro: both methods survive as distinct nodes."""
+    path = _write_py(tmp_path, '''
+        class Adapter:
+            def _get_connection(self, url):
+                return url
+
+            def get_connection(self, url):
+                return self._get_connection(url)
+
+            def unrelated(self, x):
+                return x
+    ''')
+    result = extract_python(path)
+    labels = {n["label"] for n in _rationale_free_nodes(result)}
+    assert "._get_connection()" in labels, f"private method missing: {labels}"
+    assert ".get_connection()" in labels, f"public method missing (#3302): {labels}"
+    assert ".unrelated()" in labels
+
+
+def test_leading_underscore_method_collision_ids_distinct(tmp_path: Path) -> None:
+    path = _write_py(tmp_path, '''
+        class Adapter:
+            def _get_connection(self, url):
+                return url
+
+            def get_connection(self, url):
+                return self._get_connection(url)
+    ''')
+    result = extract_python(path)
+    by_label = {n["label"]: n["id"] for n in _rationale_free_nodes(result)}
+    assert by_label["._get_connection()"] != by_label[".get_connection()"]
+
+
+def test_public_method_keeps_plain_id_private_sibling_is_salted(tmp_path: Path) -> None:
+    """The public member's id must equal what it would be with no private sibling
+    at all -- an incremental rebuild that adds/removes the private sibling must
+    not re-point edges already targeting the public method."""
+    solo = _write_py(tmp_path, '''
+        class Adapter:
+            def get_connection(self, url):
+                return url
+    ''')
+    solo_result = extract_python(solo)
+    solo_id = next(
+        n["id"] for n in _rationale_free_nodes(solo_result)
+        if n["label"] == ".get_connection()"
+    )
+
+    with_sibling = _write_py(tmp_path, '''
+        class Adapter:
+            def _get_connection(self, url):
+                return url
+
+            def get_connection(self, url):
+                return self._get_connection(url)
+    ''')
+    result = extract_python(with_sibling)
+    nodes = _rationale_free_nodes(result)
+    public_id = next(n["id"] for n in nodes if n["label"] == ".get_connection()")
+    private_id = next(n["id"] for n in nodes if n["label"] == "._get_connection()")
+
+    assert public_id == solo_id, (
+        f"adding a private sibling moved the public method's id: {solo_id} -> {public_id}"
+    )
+    assert private_id != public_id
+
+
+def test_call_edge_resolves_to_the_salted_private_method(tmp_path: Path) -> None:
+    """`get_connection`'s call to `self._get_connection(...)` must bind to the
+    salted private-method node, not dangle or bind to the public one."""
+    path = _write_py(tmp_path, '''
+        class Adapter:
+            def _get_connection(self, url):
+                return url
+
+            def get_connection(self, url):
+                return self._get_connection(url)
+    ''')
+    result = extract_python(path)
+    nodes = _rationale_free_nodes(result)
+    public_id = next(n["id"] for n in nodes if n["label"] == ".get_connection()")
+    private_id = next(n["id"] for n in nodes if n["label"] == "._get_connection()")
+
+    calls = [e for e in result["edges"] if e.get("relation") == "calls"]
+    assert (public_id, private_id) in {(e["source"], e["target"]) for e in calls}, (
+        f"no calls edge from get_connection to the salted _get_connection: {calls}"
+    )
+
+
+def test_module_level_function_collision_both_extracted(tmp_path: Path) -> None:
+    """Same bug, module-scoped (not inside a class)."""
+    path = _write_py(tmp_path, '''
+        def _helper():
+            return 1
+
+        def helper():
+            return _helper()
+    ''')
+    result = extract_python(path)
+    labels = {n["label"] for n in _rationale_free_nodes(result)}
+    assert "_helper()" in labels
+    assert "helper()" in labels
+    by_label = {n["label"]: n["id"] for n in _rationale_free_nodes(result)}
+    assert by_label["_helper()"] != by_label["helper()"]
+    calls = {(e["source"], e["target"]) for e in result["edges"] if e.get("relation") == "calls"}
+    assert (by_label["helper()"], by_label["_helper()"]) in calls
+
+
+def test_no_unique_public_member_salts_every_member(tmp_path: Path) -> None:
+    """`_x`/`__x` collide with no fully-public name in the group -- both must be
+    salted (order-independent), not one arbitrarily kept plain."""
+    path = _write_py(tmp_path, '''
+        class C:
+            def _x(self):
+                return 1
+
+            def __x(self):
+                return 2
+    ''')
+    result = extract_python(path)
+    nodes = _rationale_free_nodes(result)
+    labels = {n["label"] for n in nodes}
+    assert "._x()" in labels and ".__x()" in labels
+    ids = {n["label"]: n["id"] for n in nodes}
+    plain_class_scope_id = next(n["id"] for n in nodes if n["label"] == "C")
+    # Neither survivor kept the bare, unsalted `<class>_x` id.
+    for label in ("._x()", ".__x()"):
+        assert ids[label] != f"{plain_class_scope_id}_x", (
+            f"{label} kept the unsalted id despite no unique public member"
+        )
+
+
+def test_dunder_and_plain_name_collision_both_extracted(tmp_path: Path) -> None:
+    """`__x__`, `__x`, and `x` all strip to the same id -- the fully public `x`
+    must win and the dunder must still be extracted, salted."""
+    path = _write_py(tmp_path, '''
+        class C:
+            def __x__(self):
+                return 1
+
+            def x(self):
+                return 2
+    ''')
+    result = extract_python(path)
+    nodes = _rationale_free_nodes(result)
+    labels = {n["label"] for n in nodes}
+    assert ".__x__()" in labels and ".x()" in labels
+    ids = {n["label"]: n["id"] for n in nodes}
+    assert ids[".__x__()"] != ids[".x()"]
+
+
+def test_no_collision_ids_unaffected(tmp_path: Path) -> None:
+    """A file with no underscore-only collisions must extract exactly as before
+    -- no unnecessary salting applied to unrelated names."""
+    path = _write_py(tmp_path, '''
+        class D:
+            def public_one(self):
+                return 1
+
+            def _private_two(self):
+                return 2
+    ''')
+    result = extract_python(path)
+    nodes = _rationale_free_nodes(result)
+    ids = {n["label"]: n["id"] for n in nodes}
+    class_id = next(n["id"] for n in nodes if n["label"] == "D")
+    assert ids[".public_one()"] == f"{class_id}_public_one"
+    assert ids["._private_two()"] == f"{class_id}_private_two"
+
+
+def test_collision_in_one_class_does_not_salt_unrelated_class(tmp_path: Path) -> None:
+    """A `_foo`/`foo` collision inside class A must not touch an unrelated,
+    non-colliding `_foo` in class B (scope-keyed, not name-keyed)."""
+    path = _write_py(tmp_path, '''
+        class A:
+            def _foo(self):
+                return 1
+
+            def foo(self):
+                return self._foo()
+
+        class B:
+            def _foo(self):
+                return 3
+    ''')
+    result = extract_python(path)
+    nodes = _rationale_free_nodes(result)
+    class_b_id = next(n["id"] for n in nodes if n["label"] == "B")
+    b_foo = next(n for n in nodes if n["id"].startswith(class_b_id) and n["label"] == "._foo()")
+    assert b_foo["id"] == f"{class_b_id}_foo", (
+        "an unrelated class's non-colliding _foo was needlessly salted"
+    )