diff --git a/CHANGELOG.md b/CHANGELOG.md index 077647f13..2c456177e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ Full release notes with details on each version: [GitHub Releases](https://githu - Fix: a Java local class and a same-named external annotation (e.g. a local `class Component` and Spring's `@Component`) no longer collapse into one node (#2504, thanks @te7ina-honey). The Java type resolver now runs before the unique-label stub rewire and parks an imported-but-external type on its fully-qualified name, and cross-file import resolution checks the package. In-corpus annotation resolution is unchanged. - Fix: `graphify callflow` now respects edge direction, so the caller/callee columns are correct (#2508, thanks @Tomaskobel). The call-flow HTML loads the graph directed and recovers direction from the stored `_src`/`_tgt` markers (consistent with the `path` fix), and indirect calls are counted. - Fix: relational-intent verbs in a `query` ("calls", "uses", "extends", ...) no longer seat spurious seeds (#2507, thanks @filipechagas). Such a verb is excluded from the per-term seed guarantee, so a decoy matching only the verb no longer becomes a traversal root, while a verb that is a genuine symbol name can still be seeded on merit. +- Fix: a PHP `parent::`, `self::` or `static::` call no longer names its *scope* as the callee, so it can no longer bind to an unrelated method that merely shares that word as a label. The `scoped_call_expression` handler read the scope text as the callee name — right for `Helper::format()`, wrong for the three relative scopes: `parent::setUp()` minted a raw call to a callee literally named `parent`, and since no in-file definition answers that name, the fact reached the cross-file pass, which matches by normalized label — so it bound to whatever `->parent()` accessor the corpus happened to hold (on a 46k-node Laravel corpus, 1,698 fabricated inbound `calls` edges on one model's `parent()` accessor, credited with calling the `setUp()` of test classes across the corpus). Which class a relative scope denotes needs the inheritance context the raw-call facts do not carry, so the extractor refuses rather than guesses. Absolute scopes (`Helper::format()`, `\App\Foo::bar()`) are unaffected, and the refusal drops the fabricated edge without inventing a replacement: binding a relative scope to its real target needs the resolved base class, which is separate work. Extraction-side, so a PHP corpus needs re-extracting (`graphify update .`) to shed these edges. +- Fix: a sourceless stub can no longer shadow a real declaration in `explain`, and N stubs no longer count as one rival. `find_node_ambiguity` grouped the winning match tier by `source_file` to decide whether a tie was worth reporting, but every stub the extractor mints for a reference it could not resolve carries `source_file == ""` — so N unrelated stubs collapsed into a single `""` bucket and looked like N members of one file: no ambiguity was reported, and the caller answered with `matches[0]`, whichever stub `G.nodes()` happened to yield first. Reorder the graph and the same query answered with a different stub, equally confidently, while `graphify affected` on that same name refused with "No unique node match". Two changes fix it. `_find_node_tiers` drops sourceless nodes from the exact tier when that tier also holds a sourced one — a stub is a broken duplicate of the real declaration, never the better answer, and never something the caller could disambiguate anyway, since it has no path to retry with. And sourceless nodes are keyed individually in the ambiguity grouping rather than by their shared empty source, so a tier made *only* of stubs reports rivals instead of picking one silently. `affected`'s `resolve_seed` learns the same sourced-beats-sourceless rule in both its exact-label and bare-name passes, so the two commands now agree: both resolve to the sourced declaration when one exists, and both refuse when every rival is a stub. Sourced-vs-sourced ties are untouched, and a lone stub with no sourced rival still resolves as before. +- Fix: a PHP `interface`, `trait` or `enum` now mints a declaration node, exactly as a `class` does. `_PHP_CONFIG.class_types` held `class_declaration` alone, so no node was ever created for any of the three — 142 interfaces, 30 traits and 119 enums (291 declarations) on one 46k-node Laravel corpus. Every resolution pass that could canonicalize an edge therefore had nothing to land on, and the fan-in scattered three ways: the `implements`/`extends`/trait-`use` base minted a bare *sourceless* stub which, having an empty source key, kept the un-salted id and so shadowed the real name in `explain`; `Foo::CONST` fan-in fragmented across one salted per-file stub per referencing file (17 of them, holding 20 `references_constant` edges, for one interface); and `imports`/parameter-type `references` parked on the *file* node by the PSR-4 id-collision accident that makes `_make_id(FQN)` equal the file id — or, when the filename differs from the type name, on a sourceless FQN-labeled stub instead. The three kinds join `class_declaration` in `class_types`, mirroring Java and Groovy, which have always had `interface_declaration`. Two grammar details ride along: an enum's body is an `enum_declaration_list` rather than the `declaration_list` every other PHP declaration uses, so `body_fallback_child_types` learns it; and the `_resolve_php_type_references` raw-scan — which reads the written extends/implements/`use` text so a qualified name is resolved as written instead of guessed — scanned `class_declaration` bodies only, so `interface Reader extends Sub\Repo` and `enum Status { use Sub\Describes; }` recorded nothing and fell through to the same-namespace fallback, silently binding a rival `App\Contracts\Repo` / `App\Enums\Describes`. It now scans all four declaration kinds and both body shapes. +- Behavior change riding on the above: methods of a PHP interface, trait or enum move from file-scoped ids to type-scoped ids, and their labels gain the leading dot that a class member has always carried — `app_repo_foorepository_find` / `find()` becomes `app_repo_foorepository_foorepository_find` / `.find()`, hanging off the new declaration node rather than directly off the file. A PHP corpus must be re-extracted (`graphify update .`) to pick this up, and re-extraction is genuinely required rather than merely advisable, because AST cache entries are keyed by content hash within the version namespace — a same-version rebuild replays the pre-fix nodes untouched. `graphify update` re-extracts the whole code corpus in one pass, so it lands consistently; a *hook-driven* incremental rebuild against a graph built before this release will drop edges from files it did not re-extract to the old ids (the merge requires both endpoints live, so a stale-id edge is dropped, never repointed — loss, never misdirection), and the next full update restores them. ## 0.9.34 (2026-08-05) diff --git a/graphify/affected.py b/graphify/affected.py index ce1415223..bdcf1fde6 100644 --- a/graphify/affected.py +++ b/graphify/affected.py @@ -96,6 +96,22 @@ def _prefer_file_node( return None +def _is_sourced(graph: nx.Graph, node_id: str) -> bool: + return bool(str(graph.nodes[node_id].get("source_file") or "")) + + +def _prefer_sourced_node(graph: nx.Graph, node_ids: list[str]) -> str | None: + """Return the one node with a source file among label rivals, else None. + + Mirrors serve's `_find_node_tiers` exact-tier rule: a sourceless node is + an unresolved-reference placeholder, so a real declaration sharing its label is + the answer rather than a tie. Without this, `explain` resolved the sourced node + while `affected` refused with "No unique node match". + """ + sourced = [node_id for node_id in node_ids if _is_sourced(graph, node_id)] + return sourced[0] if len(sourced) == 1 else None + + def resolve_seed(graph: nx.Graph, query: str) -> 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 @@ -109,8 +125,17 @@ def resolve_seed(graph: nx.Graph, query: str) -> str | None: for node_id, data in graph.nodes(data=True) if _normalize_label(str(data.get("label", ""))) == query_lower ] - if len(exact_label_matches) == 1: + if len(exact_label_matches) == 1 and _is_sourced(graph, exact_label_matches[0]): return exact_label_matches[0] + if exact_label_matches: + sourced = _prefer_sourced_node(graph, exact_label_matches) + if sourced is not None: + return sourced + # A lone match that is a sourceless stub is NOT yet an answer: the real + # declaration may carry a decorated label ("handle()") that only the + # bare-name pass below reaches, and serve's exact tier — which matches + # both forms — would prefer it. That pass is a superset of this + # one, so a stub with no sourced rival still resolves there. # Callable labels are decorated ("name()"), so a bare "name" query falls # through exact matching and then ties with any "name*" sibling in the # contains pass. Match on the undecorated name before giving up. @@ -122,6 +147,10 @@ def resolve_seed(graph: nx.Graph, query: str) -> str | None: ] if len(bare_name_matches) == 1: return bare_name_matches[0] + if bare_name_matches: + sourced = _prefer_sourced_node(graph, bare_name_matches) + if sourced is not None: + return sourced exact_source_matches = [ str(node_id) for node_id, data in graph.nodes(data=True) diff --git a/graphify/extract.py b/graphify/extract.py index 8b1d3dadb..e6d100baf 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -921,7 +921,17 @@ def _get_c_func_name(node, source: bytes) -> str | None: _PHP_CONFIG = LanguageConfig( ts_module="tree_sitter_php", ts_language_fn="language_php", - class_types=frozenset({"class_declaration"}), + # `interface`, `trait` and `enum` are declaration kinds exactly like `class` + # (all four carry a `name` field and a body of member declarations), so they + # mint definition nodes too — mirroring Java (above) and Groovy. Leaving them + # out meant no PHP interface/trait/enum ever had a canonical node, so every + # resolution pass had nothing to land on: implements/mixes_in kept bare + # sourceless stubs, `Foo::CONST` fan-in fragmented across per-file stubs, and + # imports/param-type references parked on the *file* node or a FQN stub. + class_types=frozenset({ + "class_declaration", "interface_declaration", "trait_declaration", + "enum_declaration", + }), function_types=frozenset({"function_definition", "method_declaration"}), import_types=frozenset({"namespace_use_clause"}), call_types=frozenset({"function_call_expression", "member_call_expression", "scoped_call_expression", "class_constant_access_expression"}), @@ -933,7 +943,10 @@ def _get_c_func_name(node, source: bytes) -> str | None: call_accessor_node_types=frozenset({"member_call_expression"}), call_accessor_field="name", name_fallback_child_types=("name",), - body_fallback_child_types=("declaration_list", "compound_statement"), + # An enum's body is an `enum_declaration_list`, not a `declaration_list`; it + # is reachable through the `body` field, but the fallback has to know it too + # for the paths that scan children by type. + body_fallback_child_types=("declaration_list", "enum_declaration_list", "compound_statement"), function_boundary_types=frozenset({"function_definition", "method_declaration"}), import_handler=_import_php, ) diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index e19f8fe62..f4e7cbee2 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -582,6 +582,11 @@ def _php_name_text(node, source: bytes) -> str | None: return None return _read_text(node, source).rsplit("\\", 1)[-1] or None +# PHP's relative scopes. Each is resolvable only against the inheritance context +# of the class it is written in, which the raw-call facts do not carry, so none of +# them ever names a concrete callee. +_PHP_RELATIVE_SCOPE_NAMES = frozenset({"self", "static", "parent"}) + def _php_collect_type_refs(node, source: bytes, generic: bool, out: list[tuple[str, str]]) -> None: """Walk a PHP type expression; append (name, role) tuples.""" if node is None: @@ -4378,7 +4383,15 @@ def walk_calls( # Static method call: Helper::format() → callee = "Helper" scope_node = node.child_by_field_name("scope") if scope_node: - callee_name = _read_text(scope_node, source) + scope_text = _read_text(scope_node, source) + # `parent::m()` / `self::m()` / `static::m()` name no + # callee: which class the scope denotes needs inheritance + # context the raw-call facts do not carry. Naming + # the scope anyway let the cross-file label match bind + # them to any unrelated `->parent()` method in the + # corpus. Absolute scopes are unaffected. + if scope_text.lower() not in _PHP_RELATIVE_SCOPE_NAMES: + callee_name = scope_text else: # member_call_expression: $obj->method() is_member_call = True diff --git a/graphify/extractors/resolution.py b/graphify/extractors/resolution.py index b5edd264a..b66e11de6 100644 --- a/graphify/extractors/resolution.py +++ b/graphify/extractors/resolution.py @@ -2489,6 +2489,15 @@ def _external_stub(fqn: str) -> str: _PHP_SUPERTYPE_RELATIONS = ("inherits", "implements", "mixes_in") _PHP_REPOINT_RELATIONS = frozenset({"inherits", "implements", "mixes_in", "imports", "references"}) +# Every PHP declaration kind that mints a definition node (`_PHP_CONFIG.class_types`) +# and can therefore carry an extends/implements/`use`-trait clause. The raw-scan +# below used to read `class_declaration` only, so `interface Reader extends +# Sub\Repo` recorded no raw text and fell through to the same-namespace guess — +# which resolves to the wrong `Repo` when both exist. +_PHP_DECLARATION_TYPES = frozenset({ + "class_declaration", "interface_declaration", "trait_declaration", "enum_declaration", +}) + def _php_fqn_from_raw(raw: str, ns: str, uses: dict[str, str]) -> str: """Resolve a raw (possibly qualified) PHP class reference to an FQN. @@ -2617,7 +2626,7 @@ def walk(n) -> None: if c.type == "namespace_use_clause": _record_use_clause(c, prefix) return - elif t == "class_declaration": + elif t in _PHP_DECLARATION_TYPES: for child in n.children: if child.type == "base_clause": for sub in child.children: @@ -2627,7 +2636,7 @@ def walk(n) -> None: for sub in child.children: if sub.type in ("name", "qualified_name"): _record_raw("implements", _read_text(sub, source)) - elif child.type == "declaration_list": + elif child.type in ("declaration_list", "enum_declaration_list"): for member in child.children: if member.type != "use_declaration": continue diff --git a/graphify/serve.py b/graphify/serve.py index 3b205d84f..236891af4 100644 --- a/graphify/serve.py +++ b/graphify/serve.py @@ -1217,6 +1217,16 @@ def _find_node_tiers( elif term in norm_label or term in label_tokens or norm_query in norm_label: substring.append(nid) + # A sourceless node is a placeholder the extractor minted for a reference it + # could not resolve (an unresolved base type, a dangling import target). When + # a real, sourced declaration carries the same label, the stub is a broken + # duplicate of it — never the better answer, and never something the caller + # could disambiguate anyway, since it has no path to retry with. Drop the + # stubs so the exact tier holds only real declarations. + sourced_exact = [nid for nid in exact if str(G.nodes[nid].get("source_file") or "")] + if sourced_exact: + exact = sourced_exact + if source_exact: query_basename = _strip_diacritics(Path(label).name).lower() preferred = [] @@ -1258,6 +1268,19 @@ def find_node_ambiguity(G: nx.Graph, label: str) -> list[str]: tier is split that way, else `[]`. Several matches *within one file* (a file node plus its members) are ordinary precedence, not ambiguity, and return `[]`. + Sourceless nodes are the exception to the per-file grouping, in *every* tier: + each one counts as its own rival. They all carry `source_file == ""`, so keying + them by source made N unrelated stub nodes look like N members of a single + file — the 18 stubs shadowing `BalanceitemRepository` reported no ambiguity at + all, and the caller answered with `matches[0]`. + + Note the two halves of this fix have different reach. `_find_node_tiers` drops + stubs from a mixed tier only for the *exact* tier, so an exact tier arrives + here already reduced to real declarations and the per-stub keying changes + nothing for it. A winning prefix or substring tier is not reduced and can + still arrive mixed; there the keying is what stops its stubs from hiding + behind one another. + `_disambiguate_file_node_labels` (#2032) already relabels colliding *file* nodes; this covers the symbol case it does not reach. """ @@ -1267,7 +1290,8 @@ def find_node_ambiguity(G: nx.Graph, label: str) -> list[str]: by_source: dict[str, str] = {} for nid in tier: source = str(G.nodes[nid].get("source_file") or "") - by_source.setdefault(source, nid) + # "\0" can't occur in a path, so a stub never joins a real file's group. + by_source.setdefault(source or "\0" + nid, nid) return list(by_source.values()) if len(by_source) > 1 else [] return [] diff --git a/tests/test_php_declaration_nodes.py b/tests/test_php_declaration_nodes.py new file mode 100644 index 000000000..313b39dbd --- /dev/null +++ b/tests/test_php_declaration_nodes.py @@ -0,0 +1,316 @@ +"""PHP `interface` / `trait` / `enum` declarations mint canonical nodes. + +`_PHP_CONFIG.class_types` used to hold only `class_declaration`, so no node was +ever minted for a PHP interface, trait or enum. Every resolution pass that could +canonicalize an edge then had nothing to land on: the implements edge kept a bare +sourceless stub, `Foo::CONST` fan-in fragmented across per-file stubs, and +`imports`/parameter-type `references` parked on the file node (or, when the file +name differs from the type name, on a sourceless FQN-labeled stub). + +The control experiment is the shape these tests pin: change +`interface FooRepository` to `class FooRepository` and every one of those edges +canonicalizes onto the single declaration node. Interfaces, traits and enums must +behave the same way. +""" +from __future__ import annotations + +from pathlib import Path + +from graphify.extract import extract +from graphify.extractors.base import _make_id + + +def _write(path: Path, text: str) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + return path + + +def _defs(result: dict, label: str) -> list[dict]: + """Sourced (declaration) nodes carrying `label`.""" + return [n for n in result["nodes"] if n.get("label") == label and n.get("source_file")] + + +def _one_def(result: dict, label: str) -> dict: + defs = _defs(result, label) + assert len(defs) == 1, f"expected exactly one sourced `{label}` node, got {len(defs)}" + return defs[0] + + +def _node_by_id(result: dict, nid: str) -> dict | None: + return next((n for n in result["nodes"] if n.get("id") == nid), None) + + +def _edges(result: dict, relation: str) -> list[dict]: + return [e for e in result["edges"] if e.get("relation") == relation] + + +def _repro_corpus(tmp_path: Path) -> list[Path]: + """The 7-file minimal repro, verbatim in shape.""" + return [ + _write( + tmp_path / "app/Repo/FooRepository.php", + "repo = $repo;\n }\n\n" + " public function tag(): string\n {\n return FooRepository::BAR;\n }\n}\n", + ), + _write( + tmp_path / "app/Uses/Consumer2.php", + "repo = $repo;\n }\n\n" + " public function tag(): string\n {\n return FooRepository::BAR;\n }\n}\n", + ), + _write( + tmp_path / "app/Repo/Extras.php", + "value;\n }\n}\n", + ), + _write( + tmp_path / "app/Uses/OnlyImport.php", + " {e['target']}" + ) + + # Cascade A / B: no sourceless FooRepository stub survives anywhere. + stubs = [ + n for n in result["nodes"] + if not n.get("source_file") and "foorepository" in n.get("id", "").lower() + ] + assert stubs == [], f"sourceless FooRepository stubs survived: {[n['id'] for n in stubs]}" + + # The loops above filter candidates by looking their target's LABEL up in the + # node set, so an edge left pointing at a bare `foorepository` with no node at + # all resolves to no label, drops out of `landed`, and is never checked. Close + # that blind spot separately: a FooRepository edge target that names no node is + # a regression whatever else canonicalized correctly. + # + # Matched on the id's last segment, not as a substring: the bare Cascade A id + # (`foorepository`) and the Cascade B per-file salts (`_php_foorepository`) + # both end in it, while `dbfoorepository` — a different type, and the repro's + # one legitimately dangling target until the import-metadata fix lands — does not. + node_ids = {n["id"] for n in result["nodes"]} + dangling = sorted( + { + e["target"] for e in result["edges"] + if str(e.get("target", "")).lower().rsplit("_", 1)[-1] == "foorepository" + and e["target"] not in node_ids + } + ) + assert dangling == [], f"FooRepository edge targets with no node: {dangling}" + + +def test_php_trait_and_enum_declared_in_a_differently_named_file(tmp_path: Path): + # Criterion 2: `trait Loggable` and `enum Status` live in `Extras.php` + # (filename != type name), so the id-collision accident that lets a class's + # edges land on its file node cannot save them. Both need real nodes. + result = extract(_repro_corpus(tmp_path), cache_root=tmp_path) + + loggable = _one_def(result, "Loggable") + status = _one_def(result, "Status") + assert loggable["source_file"].endswith("app/Repo/Extras.php") + assert status["source_file"].endswith("app/Repo/Extras.php") + + mixes_in = _edges(result, "mixes_in") + assert mixes_in, "expected a mixes_in edge from Consumer3" + for e in mixes_in: + assert e["target"] == loggable["id"] + + imports_by_target = {e["target"] for e in _edges(result, "imports")} + assert loggable["id"] in imports_by_target, "trait import did not land on the trait node" + + # The trait's edges used to park on a sourceless `App\Repo\Loggable` FQN stub, + # because `Extras.php` cannot absorb them by the id-collision accident that + # saves a PSR-4-named class. No such stub may survive. + assert not [ + n for n in result["nodes"] + if not n.get("source_file") and n.get("label") in ("App\\Repo\\Loggable", "Loggable", "Status") + ] + + # `enum Status` is imported by Consumer3 but only used as `Status::Active`, so + # nothing mints a stub node for it and the repoint pass — which reads the + # target's stub LABEL — skips the edge. That is an independent root cause: + # the edge must be resolved from its own `target_fqn` metadata. This change's + # contract here is the half that fix needs and cannot supply: the node exists, is + # sourced, and carries the FQN the metadata will resolve to. + # + # The upstream fork selects this edge by that `target_fqn`; `_import_php` here + # stamps no metadata on a PHP `imports` edge yet, so it is selected by the bare + # short-name id it targets instead — the same edge, named the only way this + # tree can name it. + enum_import = next( + e for e in _edges(result, "imports") + if str(e.get("source_file", "")).endswith("app/Uses/Consumer3.php") + and e["target"] == _make_id("Status") + ) + assert enum_import["target"] not in {n["id"] for n in result["nodes"]}, ( + "enum import no longer dangles — the target_fqn resolution has landed; tighten this " + "assertion to `enum_import['target'] == status['id']`" + ) + + +def test_php_enum_body_members_and_clauses(tmp_path: Path): + # An enum's body is an `enum_declaration_list`, not the `declaration_list` + # every other PHP declaration uses. Its methods, its `implements` clause and + # its trait `use` must all be picked up regardless (the enum caveat). + contract = _write( + tmp_path / "app/Contracts/HasLabel.php", + "value; }\n}\n", + ) + result = extract([contract, trait, enum], cache_root=tmp_path) + + status = _one_def(result, "Status") + has_label = _one_def(result, "HasLabel") + describes = _one_def(result, "Describes") + + methods = [e for e in _edges(result, "method") if e["source"] == status["id"]] + assert [ + (_node_by_id(result, e["target"]) or {}).get("label") for e in methods + ] == [".label()"], "enum method did not attach to the enum node" + + assert [e["target"] for e in _edges(result, "implements")] == [has_label["id"]] + assert [e["target"] for e in _edges(result, "mixes_in")] == [describes["id"]] + + +def test_php_interface_extends_qualified_interface_resolves(tmp_path: Path): + # Criterion 4: the `_resolve_php_type_references` raw-scan only recognised + # `class_declaration`, so an interface's `extends` clause was never recorded. + # `interface Reader extends Sub\Repo` would then fall through to the + # same-namespace guess and bind to the WRONG `Repo`. + outer = _write( + tmp_path / "app/Contracts/Repo.php", + "parent()`` method in the corpus. + +Refusal over guessing: the relative scopes name no callee at all. Absolute +scopes (``Helper::format()``) are unaffected. +""" +from __future__ import annotations + +from pathlib import Path + +from graphify.extract import extract + + +def _calls(tmp_path: Path, files: dict[str, str]): + """Extract ``files`` (name -> source) and return ({(src, tgt): edge}, result).""" + paths = [] + for name, body in files.items(): + path = tmp_path / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body, encoding="utf-8") + paths.append(path) + result = extract(paths, cache_root=tmp_path / "graphify-out") + calls = { + (edge["source"], edge["target"]): edge + for edge in result["edges"] + if edge.get("relation") == "calls" + } + return calls, result + + +def _find(result: dict, label: str, id_contains: str) -> str: + nid = next( + ( + node["id"] + for node in result["nodes"] + if node.get("label") == label and id_contains in node["id"] + ), + None, + ) + assert nid is not None, f"no node {label}/{id_contains}" + return nid + + +def _relative_scope_corpus(scope: str, callee: str) -> dict[str, str]: + """A caller using ``::()`` plus an unrelated decoy method. + + The decoy is named after the SCOPE (``parent`` / ``self`` / ``static``) — + that is the label the buggy callee name matched against corpus-wide. + """ + return { + "app/A.php": ( + " dict: + """A sourceless stub: `_php_emit_base`'s bare shadow, or a per-file salted one. + + `omit_source_key` reproduces the attributeless node serve materializes for a + dangling edge endpoint (no `source_file` key at all, not an empty one). + """ + node = {"id": f"app_uses_consumer{index}_php_foorepository", "label": LABEL, + "community": 0} + if not omit_source_key: + node["source_file"] = "" + return node + + +def _sourced() -> dict: + return {"id": SOURCED_ID, "label": LABEL, "source_file": SOURCE_FILE, + "source_location": "L22", "community": 0} + + +def _graph_dict(nodes: list[dict]) -> dict: + return {"directed": False, "multigraph": False, "graph": {}, + "nodes": nodes, "links": []} + + +def _load(nodes: list[dict]): + return json_graph.node_link_graph( + {**_graph_dict(nodes), "directed": True}, edges="links") + + +def _write(tmp_path, nodes: list[dict], name: str = "graph.json"): + p = tmp_path / name + p.write_text(json.dumps(_graph_dict(nodes))) + return p + + +def _run_explain(monkeypatch, graph_path, label, capsys): + monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None) + monkeypatch.setattr(mainmod.sys, "argv", + ["graphify", "explain", label, "--graph", str(graph_path)]) + code = None + try: + mainmod.main() + except SystemExit as exc: + code = exc.code + return capsys.readouterr().out, code + + +# --- Q4: one sourced declaration + N sourceless stubs ------------------------ + + +@pytest.mark.parametrize("stub_count", [2, 3, 18]) +def test_sourced_node_wins_over_n_sourceless_stubs(stub_count): + """Criterion 1/4: never a silent arbitrary stub, at any stub count. + + The guard must not depend on stub counts staying high — a graph built after + the PHP declaration-node fix has fewer stubs, and the residual ones must + still be handled. + """ + stubs = [_stub(i) for i in range(stub_count)] + # Stubs first: matches[0] was a stub under graph-iteration order. + G = _load(stubs + [_sourced()]) + assert _find_node(G, LABEL)[0] == SOURCED_ID + assert find_node_ambiguity(G, LABEL) == [] + + +def test_q4_answer_does_not_depend_on_node_order(): + stubs = [_stub(i) for i in range(3)] + forward = _load(stubs + [_sourced()]) + reverse = _load([_sourced()] + list(reversed(stubs))) + assert _find_node(forward, LABEL)[0] == _find_node(reverse, LABEL)[0] == SOURCED_ID + + +def test_q4_explain_reports_the_sourced_node(monkeypatch, tmp_path, capsys): + p = _write(tmp_path, [_stub(i) for i in range(18)] + [_sourced()]) + out, code = _run_explain(monkeypatch, p, LABEL, capsys) + assert f" ID: {SOURCED_ID}" in out + assert SOURCE_FILE in out + assert "Ambiguous" not in out + assert code != 1 + + +def test_attributeless_dangling_endpoints_are_also_stubs(): + """A dangling edge endpoint has no `source_file` key at all — same rule.""" + stubs = [_stub(i, omit_source_key=True) for i in range(3)] + G = _load(stubs + [_sourced()]) + assert _find_node(G, LABEL)[0] == SOURCED_ID + assert find_node_ambiguity(G, LABEL) == [] + + +# --- all-sourceless tier: no real node to prefer, so refuse ------------------ + + +@pytest.mark.parametrize("stub_count", [2, 18]) +def test_sourceless_rivals_alone_are_ambiguous_not_an_arbitrary_pick(stub_count): + """The bug proper: N stubs shared the `""` bucket and looked unambiguous.""" + G = _load([_stub(i) for i in range(stub_count)]) + assert len(find_node_ambiguity(G, LABEL)) == stub_count + + +def test_explain_refuses_when_only_sourceless_stubs_match(monkeypatch, tmp_path, capsys): + p = _write(tmp_path, [_stub(i) for i in range(3)]) + out, code = _run_explain(monkeypatch, p, LABEL, capsys) + assert "Ambiguous" in out + assert code == 1 + assert "Node: FooRepository\n ID:" not in out + + +# --- a stub with no sourced rival is still an answer ------------------------ + + +@pytest.mark.parametrize("omit_source_key", [False, True]) +def test_lone_stub_with_no_sourced_rival_still_resolves(omit_source_key): + """The rule demotes stubs against a real declaration — it does not delete them. + + `resolve_seed`'s exact-label pass no longer returns a lone *sourceless* match + outright (a decorated "name()" declaration is invisible to that pass but + visible to the bare-name pass below it, and serve's exact tier — matching both + forms — would prefer it). A stub with nothing to lose to must therefore still + come back from a later pass rather than fall through to None. + + This asserts that outcome, not the mechanism: for an undecorated label both + the bare-name pass and the `contains` pass below it return the stub, so either + alone would satisfy this. `test_bare_name_pass_is_what_recovers_a_lone_stub` + isolates the pass the fall-through actually leans on. + """ + stub = _stub(0, omit_source_key=omit_source_key) + G = _load([stub]) + assert _find_node(G, LABEL) == [stub["id"]] + assert find_node_ambiguity(G, LABEL) == [] + assert resolve_seed(G, LABEL) == stub["id"] + + +def test_bare_name_pass_is_what_recovers_a_lone_stub(): + """A decorated sourceless stub that the `contains` pass cannot rescue. + + `handle()` never enters the exact-label pass (its stored label keeps the + decoration, the query does not), and the `contains` pass ties it with the + `handleRequest()` sibling. Only the bare-name pass sees it alone — so this is + the assertion that goes red if that pass stops returning lone matches, which + is the fall-through the exact-label change depends on. + """ + G = _load([ + {"id": "stub_handle", "label": "handle()", "source_file": "", "community": 0}, + {"id": "app_svc_php_handlerequest", "label": "handleRequest()", + "source_file": "app/Svc.php", "source_location": "L14", "community": 0}, + ]) + assert resolve_seed(G, "handle") == "stub_handle" + + +def test_lone_stub_explains_rather_than_reporting_a_phantom_ambiguity( + monkeypatch, tmp_path, capsys +): + p = _write(tmp_path, [_stub(0)]) + out, code = _run_explain(monkeypatch, p, LABEL, capsys) + assert f" ID: {_stub(0)['id']}" in out + assert "Ambiguous" not in out + assert code != 1 + + +# --- Q2: one sourced + one sourceless --------------------------------------- + + +def test_q2_shape_does_not_regress(): + """Criterion 2: resolve to the sourced node (never the stub, never silent).""" + G = _load([_stub(0), _sourced()]) + assert _find_node(G, LABEL)[0] == SOURCED_ID + assert find_node_ambiguity(G, LABEL) == [] + + +def test_q2_explain_reports_the_sourced_node(monkeypatch, tmp_path, capsys): + p = _write(tmp_path, [_stub(0), _sourced()]) + out, code = _run_explain(monkeypatch, p, LABEL, capsys) + assert f" ID: {SOURCED_ID}" in out + assert code != 1 + + +# --- explain / affected consistency (criterion 3) --------------------------- + + +@pytest.mark.parametrize("stub_count", [1, 2, 18]) +def test_explain_and_affected_agree_when_a_sourced_node_exists(stub_count): + """`resolve_seed` refused with "No unique node match" while `explain` + answered — that divergence is the reported symptom.""" + G = _load([_stub(i) for i in range(stub_count)] + [_sourced()]) + assert resolve_seed(G, LABEL) == _find_node(G, LABEL)[0] == SOURCED_ID + + +@pytest.mark.parametrize("stub_count", [2, 18]) +def test_explain_and_affected_both_refuse_when_every_rival_is_sourceless(stub_count): + G = _load([_stub(i) for i in range(stub_count)]) + assert resolve_seed(G, LABEL) is None + assert find_node_ambiguity(G, LABEL) # explain refuses too + + +def test_affected_prefers_sourced_node_for_a_callable_label(): + """`resolve_seed`'s bare-name pass (decorated "name()" labels) needs the + same rule as its exact-label pass.""" + nodes = [ + {"id": "stub_handle", "label": "handle", "source_file": "", "community": 0}, + {"id": "app_svc_php_handle", "label": "handle()", + "source_file": "app/Svc.php", "source_location": "L9", "community": 0}, + ] + G = _load(nodes) + assert resolve_seed(G, "handle") == "app_svc_php_handle" + + +# --- the pre-existing monorepo tie must still be reported ------------------- + + +def test_two_sourced_rivals_are_still_ambiguous(): + """Sourced-vs-sourced (#2032's symbol case) is untouched by the stub rule.""" + G = _load([ + {"id": "chat_port", "label": "MetricsPort", + "source_file": "services/chat/ports/metrics.port.ts", "community": 0}, + {"id": "scrape_port", "label": "MetricsPort", + "source_file": "services/scraping/ports/metrics.port.ts", "community": 0}, + ]) + assert len(find_node_ambiguity(G, "MetricsPort")) == 2