Repository navigation
Typed-relationship queries match relationship types the writer never emits (:INHERITS, :OVERRIDES, :TYPES, :CAPTURES) #61
Description
Filing this at @lexasub's request from #50 — the follow-up to the :EDGE mismatch that PR fixed for the call path only.
Summary
batch_upsert_edges writes every edge as a single untyped :EDGE relationship and stores the kind as a property. A number of read sites instead match on typed relationships (:INHERITS, :OVERRIDES, :TYPES, :CAPTURES, :EXTENDS, :IMPLEMENTS) that nothing ever creates, so those queries can only ever return empty — silently, with no error.
The writer
# ast_rag/repositories/queries.py:58
MERGE (a)-[r:EDGE {id: e.id}]->(b)
SET r += eEverything lands on :EDGE; kind arrives as a property via SET r += e. The only other relationship types created anywhere are :RELATES (neo4j_repository.py:329,360) and :CONTAINS_BLOCK (graph_updater_service.py:857) — both of which do have matching readers and are fine.
There is no dynamic/APOC relationship-type creation, so nothing else can be emitting these types.
The broken read sites
Eight query sites match relationship types that no writer emits:
| file:line | pattern | result |
|---|---|---|
api/ast_rag_api.py:470 |
[:INHERITS|EXTENDS|IMPLEMENTS*1..N] |
always empty |
api/ast_rag_api.py:486 |
[:INHERITS|EXTENDS|IMPLEMENTS*1..N] |
always empty |
api/ast_rag_api.py:520 |
[:OVERRIDES*1..N] |
always empty |
api/ast_rag_api.py:908 |
[r:TYPES] |
always 0 |
api/ast_rag_api.py:913 |
[r:INHERITS|EXTENDS|IMPLEMENTS] |
always 0 |
api/ast_rag_api.py:981 |
[r:TYPES] |
always 0 |
api/ast_rag_api.py:1005 |
[r:INHERITS|EXTENDS|IMPLEMENTS] |
always 0 |
services/search_service.py:499 |
[:CAPTURES] |
always empty |
This affects get_inheritance_tree / ancestor–descendant traversal, find_overrides, and the type-usage and inheritance counts in the node-detail/impact paths.
Clearest illustration
Both shapes sit in the same method, a few lines apart — the first works, the next two cannot:
-- ast_rag_api.py:903 WORKS (the shape #50 moved the call path to)
MATCH (caller)-[r:EDGE]->(target {id: $node_id})
WHERE caller.valid_to IS NULL AND r.valid_to IS NULL AND r.kind IN $call_kinds
RETURN count(*) as count
-- ast_rag_api.py:908 ALWAYS 0
MATCH (user)-[r:TYPES]->(target {id: $node_id})
WHERE user.valid_to IS NULL AND r.valid_to IS NULL
RETURN count(*) as count
-- ast_rag_api.py:913 ALWAYS 0
MATCH (child)-[r:INHERITS|EXTENDS|IMPLEMENTS]->(parent {id: $node_id})
WHERE child.valid_to IS NULL AND r.valid_to IS NULL
RETURN count(*) as countTwo ways to fix it — your call
Option A — align the readers to the writer. Rewrite the eight sites as [r:EDGE] + WHERE r.kind IN [...], matching what #50 did for calls. Small, contained, no re-index required, and consistent with the majority of existing queries. The cost is that variable-length traversals get more awkward: [:INHERITS*1..N] becomes [r:EDGE*1..N] with an ALL(rel IN r WHERE rel.kind IN [...]) predicate, which Neo4j cannot index as well.
Option B — make the writer emit typed relationships. Keeps the traversal queries natural and lets Neo4j use relationship-type indexes, which matters for the *1..N inheritance walks. Costs a dynamic write (APOC or a per-kind MERGE branch), and it is a breaking graph-schema change — existing indexes need re-indexing.
I lean A for now: it is the smaller change, it makes the queries correct today, and it does not force a re-index on anyone. B is the better long-term shape if inheritance traversal depth becomes a performance concern — worth doing deliberately rather than as a bug fix.
Happy to send a PR for whichever you prefer. If A, I'd do it as one PR across the eight sites with a test per query path asserting a non-empty result on a fixture that actually has inheritance/override/type edges — the current tests don't catch this because they never assert these paths return anything.
Metadata
Metadata
Assignees
Labels
Projects
- StatusShow more project fieldsDone
Activity
r0h1tb commentedon Aug 2, 2026
Heads-up @lexasub: I merged #48, #56 and #60 today using the write access you gave me, rather than leaving them sitting. Flagging it explicitly since you've reviewed and merged every previous one yourself — if you'd rather I keep sending them for review instead of self-merging, just say so and I will.
All three were red only because they predated #51's ruff pin. Rebasing onto
mainwas the entire fix; no behaviour was changed to make CI pass. Each was verified locally against the full suite before merging, andmainis green atff8cf65.The one judgement call worth your eyes is in #56: it still drove the index loop with
console.status, but #54 had landed_index_progress()bars onmainin the meantime. I kept your merged progress bars and moved the two-phase index inside them, and converted the new reference-resolution phase to a bar as well. Easy to change if you'd prefer a spinner there.This issue is still the open question — whichever way you want to go on
:EDGEvs. typed relationships, I'll send the PR.lexasub commentedon Aug 2, 2026
I wanted to test things this week and review the PR. Yes, please send the PRs - I’ll take a look. If I immediately agree with the changes, I’ll merge them, provided the PR isn’t a draft.
Alternatively, I can approve the PR, and you can handle the merge - in case you want to make further adjustments later.
Yes, I’m fine with merging without a review, but only if you’re 100 % sure the MR is perfect and there won’t be any questions about different implementation approaches. Or if the feature is urgently needed right now and it’s critical to merge it as a top priority, ahead of everything else.
lexasub commentedon Aug 4, 2026
@r0h1tb option a - fast victory (also may document in comments about option b)
docs(builds): record production-readiness run (audit, E2E verify, sig…