fix(search): consume bfs relationship hits directly in edge_bfs_search#1653
Open
kramerica-inc-dev wants to merge 1 commit into
Open
fix(search): consume bfs relationship hits directly in edge_bfs_search#1653kramerica-inc-dev wants to merge 1 commit into
kramerica-inc-dev wants to merge 1 commit into
Conversation
Completes getzep#1500 on the generic edge_bfs_search path in search_utils.py, which is what executes today for FalkorDB and Neo4j: driver.search_interface is never assigned, so the driver-level operations modules getzep#1500 patches are not reached at runtime. Mirrors the exact transform getzep#1500 applies to the FalkorDB driver copy of this query: replace the per-row re-MATCH by uuid (MATCH (n:Entity)-[e:RELATES_TO {uuid: rel.uuid}]-(m:Entity)) with a direct WITH over the relationship already produced by UNWIND relationships(path), deriving endpoints via startNode/endNode. The explicit type(e) = 'RELATES_TO' guard is required because the BFS pattern traverses RELATES_TO|MENTIONS and the old re-MATCH filtered MENTIONS out implicitly. As a side effect this also removes the duplicated/swapped source-target rows on this path (the old undirected re-MATCH matched each edge twice; see getzep#789 and PR getzep#1436): startNode/endNode yields one row per relationship in its original orientation. getzep#789 also flags get_embeddings_for_edges, which is out of scope here. Refs getzep#1272, getzep#1500. Related to getzep#789, getzep#1436.
Contributor
|
I have read the CLA Document and I hereby sign the CLA behalf on myself, e-mail: example@example.com or I have read the CLA Document and I hereby sign the CLA behalf of my company, e-mail: example@example.com Signature is valid for 6 months. This bot will be retriggered when the Contributor License Agreement comment has been provided. Posted by the CLA Assistant Lite bot. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(search): consume bfs relationship hits directly in edge_bfs_search
Summary
Completes #1500 on the path that executes today. #1500 rewrites the edge fulltext and BFS queries to consume the relationships the index/path already returned (
startNode/endNode) instead of re-matching every hit by uuid against the whole graph — but its BFS hunk only lands ingraphiti_core/driver/falkordb/operations/search_ops.py. That module is not reached at runtime:driver.search_interfaceis declared (graphiti_core/driver/driver.py) and never assigned anywhere ingraphiti_core, soedge_bfs_searchalways falls through to the generic query ingraphiti_core/search/search_utils.py. This PR applies the exact same transform there:Notes on the transform:
RELATES_TO|MENTIONS, sorelcan be a MENTIONS edge; the old re-MATCH filtered those out implicitly via thee:RELATES_TOlabel. TheWHERE type(e) = 'RELATES_TO'guard preserves that (same as fix: avoid O(matches×graph) re-MATCH in edge search #1500's BFS hunk). The trailingWITH e, n, mkeeps the appended filter clause (WHERE …) valid and thee/n/mbindings, filter constructor, andRETURNuntouched.RETURN DISTINCTcould not collapse the swapped source/target rows.startNode/endNodeyields one row per relationship in its original orientation, so this also removes the duplicated/swapped-edge symptom reported in [BUG] BFS Returns Duplicate Edges with Swapped Source/Target Nodes #789 and targeted by PR fix: use directed match in edge_bfs_search to prevent duplicate edges #1436 — on this path. [BUG] BFS Returns Duplicate Edges with Swapped Source/Target Nodes #789 additionally flags the same undirected pattern inget_embeddings_for_edges, which is out of scope here, so this PR does not claim to close it.startNode()/endNode()are native on both engines that share this branch — Neo4j and FalkorDB (the Neptune branch in this same function already uses them in itsRETURN).Type of Change
Objective
The re-MATCH forces an O(matches × graph) label scan per BFS hit instead of consuming the relationship the path traversal already produced. On our production FalkorDB graph (~2.6k Entity nodes), removing this same antipattern on the fulltext path took dense-document episode processing from 15–25 min down to 1–4 min;
GRAPH.PROFILEof the rewritten query resolves 572 hits in <2 ms with no scan operator (measured 2026-07-14). The BFS variant fixed here is the same shape, made worse by the undirected match doubling every row.Testing
Added
tests/utils/search/test_edge_bfs_query_shape.py, a DB-free query-shape regression test mirroring the RecordingExecutor approach of #1500'stests/test_falkordb_search_ops.py: a recording driver captures the emitted Cypher. Asserts: nouuid: rel.uuidre-MATCH in the BFS query;startNode/endNodeconsumption present; explicittype(e) = 'RELATES_TO'guard present;e/n/mbindings still reach filters andRETURN; and the fulltext query is not rewritten by this change (formulated so it stays green after #1500 merges).Breaking Changes
No. Query results are identical except that the duplicated/swapped-orientation rows produced by the old undirected re-MATCH are gone; each edge is now returned once, in its stored direction.
Checklist
make lintpasses —ruff format --check+ruff checkclean on changed files)Related Issues
Refs #1272, #1500 (completes it on the live generic path). Related to #789 and PR #1436 (resolves the duplicate/swapped-edge symptom for
edge_bfs_search;get_embeddings_for_edgesfrom #789 remains).