From cf381a873fceffbbc87146830e3196498bfae2ae Mon Sep 17 00:00:00 2001 From: Rohit Behera <126186063+r0h1tb@users.noreply.github.com> Date: Sat, 8 Aug 2026 15:13:48 +0530 Subject: [PATCH] fix(queries): match :EDGE and filter on kind in typed traversals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit batch_upsert_edges writes every edge as a single untyped :EDGE relationship and keeps the semantic kind as a property: MERGE (a)-[r:EDGE {id: e.id}]->(b) SET r += e Eight read sites instead matched relationship types that no writer creates (:INHERITS, :EXTENDS, :IMPLEMENTS, :OVERRIDES, :TYPES, :CAPTURES). Those queries are valid Cypher over an empty set: they compile, they run, and they return nothing, for ever, with no error. That silently emptied find_subclasses, find_superclasses, find_overrides and the type-usage and inheritance counts in the node-detail and impact paths. This is option A from #61, as chosen on the issue: align the readers to the writer. Each site now matches [r:EDGE] and filters with `WHERE r.kind IN $...`, the shape #50 already moved the call path to. Variable-length traversals use `all(rel IN rels WHERE rel.kind IN $...)`, matching find_callers/find_callees. Option B — emitting real typed relationships — keeps traversals natural and lets Neo4j use relationship-type indexes for the *1..N inheritance walks, but needs a dynamic write and a re-index. Documented in a comment next to the new kind groups; worth doing deliberately rather than as a bug fix. Two details worth calling out: - The paginated inheritance query returned `type(r) as rel_type`, which would now always be "EDGE". It returns `r.kind` instead, so the reported reference type still says INHERITS/EXTENDS/IMPLEMENTS. - The :CAPTURES site is corrected for consistency but still returns nothing, for a different reason: the parsing layer emits no CAPTURES edges or Variable nodes yet. EdgeKind.CAPTURES is declared so the kind is no longer a bare undeclared string. The regression test is static, for the same reason test_query_parameters_bound is: proving a traversal returns rows needs a populated graph with real inheritance and override edges. It collects the relationship types the writers create and the ones the readers match, and asserts the second set is a subset of the first — so a future typed writer makes its matching readers pass on their own. Fixes #61 --- ast_rag/api/ast_rag_api.py | 73 +++++++++++++----- ast_rag/dto/enums.py | 1 + ast_rag/services/search_service.py | 17 +++- tests/test_relationship_types_emitted.py | 98 ++++++++++++++++++++++++ 4 files changed, 168 insertions(+), 21 deletions(-) create mode 100644 tests/test_relationship_types_emitted.py diff --git a/ast_rag/api/ast_rag_api.py b/ast_rag/api/ast_rag_api.py index 3350e5e..bed2a37 100644 --- a/ast_rag/api/ast_rag_api.py +++ b/ast_rag/api/ast_rag_api.py @@ -52,6 +52,15 @@ # traversals must filter on that property rather than on a relationship type. CALL_EDGE_KINDS = ["CALLS", "VIRTUAL_CALL", "LAMBDA_CALL", "CROSS_FILE_CALL"] +# Edges are all written as a single untyped ``:EDGE`` relationship with the +# semantic kind kept on the ``kind`` property (see ``batch_upsert_edges``), so +# every traversal matches ``:EDGE`` and filters on these groups. Matching a +# typed relationship such as ``[:INHERITS]`` compiles but can never return a +# row, because no writer creates one. See #61. +INHERITANCE_EDGE_KINDS = ["INHERITS", "EXTENDS", "IMPLEMENTS"] +OVERRIDE_EDGE_KINDS = ["OVERRIDES"] +TYPE_EDGE_KINDS = ["TYPES"] + logger = logging.getLogger(__name__) # The active version filter used in all Cypher queries @@ -467,15 +476,18 @@ def find_subclasses(self, node_id: str, max_depth: int = 3) -> list[ASTNode]: """Find all subclasses/implementors of the given type.""" max_depth = min(max_depth, 5) cypher = f""" -MATCH (child)-[:INHERITS|EXTENDS|IMPLEMENTS*1..{max_depth}]->(parent {{id: $node_id}}) +MATCH (child)-[rels:EDGE*1..{max_depth}]->(parent {{id: $node_id}}) WHERE child.valid_to IS NULL + AND all(rel IN rels WHERE rel.kind IN $inheritance_kinds) RETURN DISTINCT child ORDER BY child.qualified_name LIMIT 100 """ results: list[ASTNode] = [] with self._driver.session() as session: - for record in session.run(cypher, node_id=node_id): + for record in session.run( + cypher, node_id=node_id, inheritance_kinds=INHERITANCE_EDGE_KINDS + ): results.append(_record_to_node(dict(record["child"]))) return results @@ -483,15 +495,18 @@ def find_superclasses(self, node_id: str, max_depth: int = 3) -> list[ASTNode]: """Find all parent classes/interfaces of the given type.""" max_depth = min(max_depth, 5) cypher = f""" -MATCH (child {{id: $node_id}})-[:INHERITS|EXTENDS|IMPLEMENTS*1..{max_depth}]->(parent) +MATCH (child {{id: $node_id}})-[rels:EDGE*1..{max_depth}]->(parent) WHERE parent.valid_to IS NULL + AND all(rel IN rels WHERE rel.kind IN $inheritance_kinds) RETURN DISTINCT parent ORDER BY parent.qualified_name LIMIT 100 """ results: list[ASTNode] = [] with self._driver.session() as session: - for record in session.run(cypher, node_id=node_id): + for record in session.run( + cypher, node_id=node_id, inheritance_kinds=INHERITANCE_EDGE_KINDS + ): results.append(_record_to_node(dict(record["parent"]))) return results @@ -517,8 +532,9 @@ def find_overrides(self, method_node_id: str, max_depth: int = 3) -> list[ASTNod cypher = f""" MATCH (base {{id: $method_node_id}}) WHERE base.kind IN ['Method', 'Function'] -MATCH (overrider)-[:OVERRIDES*1..{max_depth}]->(base) +MATCH (overrider)-[rels:EDGE*1..{max_depth}]->(base) WHERE overrider.valid_to IS NULL + AND all(rel IN rels WHERE rel.kind IN $override_kinds) RETURN DISTINCT overrider ORDER BY overrider.qualified_name LIMIT 100 @@ -526,7 +542,9 @@ def find_overrides(self, method_node_id: str, max_depth: int = 3) -> list[ASTNod results: list[ASTNode] = [] with self._driver.session() as session: - for record in session.run(cypher, method_node_id=method_node_id): + for record in session.run( + cypher, method_node_id=method_node_id, override_kinds=OVERRIDE_EDGE_KINDS + ): node_data = dict(record["overrider"]) results.append(_record_to_node(node_data)) @@ -913,13 +931,13 @@ def _count_usages_of_node(self, node_id: str) -> int: RETURN count(*) as count """ types_count_cypher = """ -MATCH (user)-[r:TYPES]->(target {id: $node_id}) -WHERE user.valid_to IS NULL AND r.valid_to IS NULL +MATCH (user)-[r:EDGE]->(target {id: $node_id}) +WHERE user.valid_to IS NULL AND r.valid_to IS NULL AND r.kind IN $type_kinds RETURN count(*) as count """ inherits_count_cypher = """ -MATCH (child)-[r:INHERITS|EXTENDS|IMPLEMENTS]->(parent {id: $node_id}) -WHERE child.valid_to IS NULL AND r.valid_to IS NULL +MATCH (child)-[r:EDGE]->(parent {id: $node_id}) +WHERE child.valid_to IS NULL AND r.valid_to IS NULL AND r.kind IN $inheritance_kinds RETURN count(*) as count """ total = 0 @@ -928,11 +946,13 @@ def _count_usages_of_node(self, node_id: str) -> int: record = result.single() total += record["count"] if record else 0 - result = session.run(types_count_cypher, node_id=node_id) + result = session.run(types_count_cypher, node_id=node_id, type_kinds=TYPE_EDGE_KINDS) record = result.single() total += record["count"] if record else 0 - result = session.run(inherits_count_cypher, node_id=node_id) + result = session.run( + inherits_count_cypher, node_id=node_id, inheritance_kinds=INHERITANCE_EDGE_KINDS + ) record = result.single() total += record["count"] if record else 0 @@ -992,14 +1012,20 @@ def _find_usages_of_node_paginated( # Query for incoming TYPES edges with pagination types_cypher = """ -MATCH (user)-[r:TYPES]->(target {id: $node_id}) -WHERE user.valid_to IS NULL AND r.valid_to IS NULL +MATCH (user)-[r:EDGE]->(target {id: $node_id}) +WHERE user.valid_to IS NULL AND r.valid_to IS NULL AND r.kind IN $type_kinds RETURN user, r ORDER BY user.qualified_name SKIP $offset LIMIT $limit """ with self._driver.session() as session: - for record in session.run(types_cypher, node_id=node_id, offset=offset, limit=limit): + for record in session.run( + types_cypher, + node_id=node_id, + offset=offset, + limit=limit, + type_kinds=TYPE_EDGE_KINDS, + ): user_data = dict(record["user"]) edge_data = dict(record["r"]) references.append( @@ -1015,15 +1041,24 @@ def _find_usages_of_node_paginated( ) # Query for incoming INHERITS/EXTENDS/IMPLEMENTS edges with pagination + # ``type(r)`` is now always "EDGE", so the reported reference type comes + # from the ``kind`` property instead -- it is what carries the semantic + # relationship (INHERITS / EXTENDS / IMPLEMENTS). inherits_cypher = """ -MATCH (child)-[r:INHERITS|EXTENDS|IMPLEMENTS]->(parent {id: $node_id}) -WHERE child.valid_to IS NULL AND r.valid_to IS NULL -RETURN child, r, type(r) as rel_type +MATCH (child)-[r:EDGE]->(parent {id: $node_id}) +WHERE child.valid_to IS NULL AND r.valid_to IS NULL AND r.kind IN $inheritance_kinds +RETURN child, r, r.kind as rel_type ORDER BY child.qualified_name SKIP $offset LIMIT $limit """ with self._driver.session() as session: - for record in session.run(inherits_cypher, node_id=node_id, offset=offset, limit=limit): + for record in session.run( + inherits_cypher, + node_id=node_id, + offset=offset, + limit=limit, + inheritance_kinds=INHERITANCE_EDGE_KINDS, + ): child_data = dict(record["child"]) edge_data = dict(record["r"]) references.append( diff --git a/ast_rag/dto/enums.py b/ast_rag/dto/enums.py index 08ac14c..79985db 100644 --- a/ast_rag/dto/enums.py +++ b/ast_rag/dto/enums.py @@ -57,6 +57,7 @@ class EdgeKind(str, Enum): OVERRIDES = "OVERRIDES" DEPENDS_ON = "DEPENDS_ON" TYPES = "TYPES" + CAPTURES = "CAPTURES" # Lambda/closure captures a variable VIRTUAL_CALL = "VIRTUAL_CALL" LAMBDA_CALL = "LAMBDA_CALL" CROSS_FILE_CALL = "CROSS_FILE_CALL" diff --git a/ast_rag/services/search_service.py b/ast_rag/services/search_service.py index 028b513..6e21c13 100644 --- a/ast_rag/services/search_service.py +++ b/ast_rag/services/search_service.py @@ -16,6 +16,11 @@ logger = logging.getLogger(__name__) +# Edges are written as a single untyped ``:EDGE`` relationship with the +# semantic kind on the ``kind`` property, so traversals match ``:EDGE`` and +# filter on the kind. See ``batch_upsert_edges`` and #61. +CAPTURE_EDGE_KINDS = ["CAPTURES"] + class SearchService: """Service for unified code search operations. @@ -484,7 +489,8 @@ def find_lambdas( List of lambda blocks with captured variables """ conditions = ["b.valid_to IS NULL", "b.block_type = 'lambda'"] - params: dict[str, str] = {} + # Values are not all strings: the capture-kind filter binds a list. + params: dict[str, object] = {} if lang: conditions.append("b.lang = $lang") @@ -493,15 +499,22 @@ def find_lambdas( where_clause = " AND ".join(conditions) if with_captured_vars: + # Edges are stored as :EDGE with the semantic kind on `kind`, so + # matching [:CAPTURES] could never bind. Note this stays empty for + # a second reason: the parsing layer does not emit CAPTURES edges + # or Variable nodes yet, so `captured_vars` is [] until it does. + # The shape is corrected here so it works once that lands. See #61. cypher = f""" MATCH (b:Block) WHERE {where_clause} -OPTIONAL MATCH (b)-[:CAPTURES]->(v:Variable) +OPTIONAL MATCH (b)-[r:EDGE]->(v:Variable) +WHERE r.kind IN $capture_kinds AND r.valid_to IS NULL WITH b, collect(v.name) as captured_vars RETURN b, captured_vars ORDER BY b.file_path, b.start_line LIMIT $limit """ + params["capture_kinds"] = CAPTURE_EDGE_KINDS else: cypher = f""" MATCH (b:Block) diff --git a/tests/test_relationship_types_emitted.py b/tests/test_relationship_types_emitted.py new file mode 100644 index 0000000..388224c --- /dev/null +++ b/tests/test_relationship_types_emitted.py @@ -0,0 +1,98 @@ +"""Every relationship type a query matches must be one some writer emits. + +``batch_upsert_edges`` stores *all* edges as a single untyped ``:EDGE`` +relationship and keeps the semantic kind as a property:: + + MERGE (a)-[r:EDGE {id: e.id}]->(b) + SET r += e + +A query that instead matches ``[:INHERITS]`` or ``[r:TYPES]`` is therefore +valid Cypher over an empty set: it compiles, it runs, and it returns nothing, +for ever, with no error. Eight read sites did exactly that, so +``get_inheritance_tree``, ``find_overrides`` and the type-usage and inheritance +counts in the node-detail and impact paths silently reported nothing. + +A runtime test is a poor fit for the same reason it was for +``test_query_parameters_bound``: proving a traversal returns rows needs a +populated graph with real inheritance and override edges. This checks the +invariant statically instead -- collect the relationship types the writers +create, collect the ones the readers match, and assert the second set is a +subset of the first. + +The check is deliberately derived from the source rather than hard-coded: if a +writer is later changed to emit typed relationships (option B in #61), the +matching readers stop failing this test on their own. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +PACKAGE = Path(__file__).parent.parent / "ast_rag" + +# -[var:TYPE]-, -[:A|B|C*1..3]-, -[rels:EDGE*1..{max_depth}]- ... +RELATIONSHIP = re.compile(r"-\[\s*\w*\s*:\s*([A-Z_][A-Z0-9_]*(?:\s*\|\s*[A-Z_][A-Z0-9_]*)*)") + +# A Cypher clause that *creates* a relationship rather than matching one. +WRITES = re.compile(r"\b(?:MERGE|CREATE)\b") +READS = re.compile(r"\bMATCH\b") + + +def _types(fragment: str) -> set[str]: + """Split an alternation such as ``INHERITS|EXTENDS`` into its types.""" + return {t.strip() for t in fragment.split("|") if t.strip()} + + +def _scan() -> tuple[set[str], dict[str, list[str]]]: + """Return (types written, {type: [where it is matched]}). + + Cypher is embedded as string literals, including f-strings, so this reads + the source as text. A relationship pattern is attributed to the nearest + preceding clause keyword on the same line, which is how these queries are + written throughout the package. + """ + written: set[str] = set() + matched: dict[str, list[str]] = {} + + for path in sorted(PACKAGE.rglob("*.py")): + for lineno, line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1): + found = RELATIONSHIP.findall(line) + if not found: + continue + where = f"{path.relative_to(PACKAGE.parent)}:{lineno}" + for fragment in found: + types = _types(fragment) + if WRITES.search(line): + written |= types + elif READS.search(line): + for t in types: + matched.setdefault(t, []).append(where) + + return written, matched + + +def test_writers_emit_at_least_one_relationship_type(): + """Guard the guard: if this finds nothing, the scan itself is broken.""" + written, _ = _scan() + assert "EDGE" in written, ( + "no writer emitting :EDGE was found -- the source scan is not working, " + f"so the subset check below proves nothing. Found: {sorted(written)}" + ) + + +def test_every_matched_relationship_type_is_emitted_somewhere(): + written, matched = _scan() + + unmatchable = {t: sites for t, sites in matched.items() if t not in written} + + assert not unmatchable, ( + "These queries match relationship types no writer creates, so they can " + "only ever return empty:\n" + + "\n".join( + f" :{t} matched at {', '.join(sites)}" for t, sites in sorted(unmatchable.items()) + ) + + f"\n\nRelationship types actually written: {sorted(written)}." + "\nEdges are stored as :EDGE with the semantic kind on the `kind` " + "property, so match [r:EDGE] and filter with `WHERE r.kind IN [...]`." + )