From 523b02083f25b918cfdf17a53590f6fda3355cdf Mon Sep 17 00:00:00 2001 From: KaifAhmad1 Date: Thu, 16 Apr 2026 15:14:17 +0530 Subject: [PATCH] feat(explorer): add bidirectional path finding with directed=false param (#469) - PathFinder.bfs_shortest_path() and dijkstra_shortest_path() gain a directed: bool = True parameter. When False, a temporary undirected view (graph.to_undirected()) is used for traversal only; the original directed edges are preserved and returned in the response. - _make_undirected_view() helper added to PathFinder; falls back safely for non-NetworkX graph types. - GET /api/graph/node/{id}/path exposes ?directed=false query param. - PathResponse gains a directed: bool field echoing the mode used. - Route now returns 404 on empty path (previously returned 200 with path: []). - 21 new tests: 12 unit (TestBidirectionalPathFinding) + 9 API-level (TestBidirectionalPathRoute). All 120 tests pass. - CHANGELOG updated under [Unreleased]. Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 3 +- semantica/explorer/routes/graph.py | 7 +- semantica/explorer/schemas.py | 1 + semantica/kg/path_finder.py | 57 ++++++++----- tests/explorer/test_explorer_api.py | 120 ++++++++++++++++++++++++++++ tests/kg/test_path_finder.py | 84 +++++++++++++++++++ 6 files changed, 251 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92c9811b..d00c2d11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -- **Enhancement: Native `KnowledgeGraph` type support in `KGVisualizer`** (PR `kg` by @KaifAhmad1, closes #471): Added `semantica/kg/knowledge_graph.py` — a formal `KnowledgeGraph` dataclass (`entities`, `relationships`, `metadata`) that is now the canonical in-memory type produced and consumed by the Semantica KG pipeline. Exported from `semantica.kg`. `KGVisualizer` gains `_convert_knowledge_graph()` — an authoritative, non-mutating conversion path from `KnowledgeGraph` to the internal dict format — and `_normalize_graph()` now routes `isinstance(graph, KnowledgeGraph)` through it as an explicit fast-path before duck-typing. All five public entry points (`visualize_network`, `visualize_communities`, `visualize_centrality`, `visualize_entity_types`, `visualize_relationship_matrix`) accept `KnowledgeGraph` directly; no manual conversion required. All existing callers passing dicts or duck-typed objects are unaffected. 15 new tests in `TestFormalKnowledgeGraphType` (conversion shape, non-mutation, determinism, routing, all five entry points, import availability). +- **Enhancement: Bidirectional path finding in Knowledge Explorer** (closes #469 by @KaifAhmad1): Path queries in the Explorer were direction-sensitive — querying B→A when only the edge A→B existed always returned no result, because `PathFinder._get_neighbors()` called `graph.neighbors(node)` which on a `nx.DiGraph` yields only successors. Added a `directed: bool = True` parameter to `bfs_shortest_path()` and `dijkstra_shortest_path()`. When `directed=False` a lightweight undirected view is built via `graph.to_undirected()` for the traversal pass only; the original directed edges are preserved and returned in the response. A `_make_undirected_view()` helper encapsulates the conversion and falls back safely for non-NetworkX graph types. The `/api/graph/node/{id}/path` route exposes the parameter as a query string flag (`?directed=false`); `PathResponse` gains a `directed: bool` field that echoes the mode used. Default is `True`, so all existing callers are unaffected. The route also gained an empty-path 404 guard — previously a traversal that found no path returned `200` with `path: []` instead of `404`. 21 new tests: 12 unit tests in `TestBidirectionalPathFinding` (`tests/kg/test_path_finder.py`) and 9 API-level tests in `TestBidirectionalPathRoute` (`tests/explorer/test_explorer_api.py`). +- **Enhancement: Native `KnowledgeGraph` type support in `KGVisualizer`** (PR `kg` by @KaifAhmad1, closes #471): Added `semantica/kg/knowledge_graph.py` — a formal `KnowledgeGraph` dataclass (`entities`, `relationships`, `metadata`) that is now the canonical in-memory type produced and consumed by the Semantica KG pipeline. Exported from `semantica.kg`. `KGVisualizer` gains `_convert_knowledge_graph()` — an authoritative, non-mutating conversion path from `KnowledgeGraph` to the internal dict format — and `_normalize_graph()` now routes `isinstance(graph, KnowledgeGraph)` through it as an explicit fast-path before duck-typing. All five public entry points (`visualize_network`, `visualize_communities`, `visualize_centrality`, `visualize_entity_types`, `visualize_relationship_matrix`) accept `KnowledgeGraph` directly; no manual conversion required. All existing callers passing dicts or duck-typed objects are unaffected. 15 new tests in `TestFormalKnowledgeGraphType` (conversion shape, non-mutation, determinism, routing, all five entry points, import availability). - **Fix: `KGVisualizer` now accepts `KnowledgeGraph` objects in all `visualize_*` methods** (PR `visualization` by @KaifAhmad1, closes #458): All five public methods (`visualize_network`, `visualize_communities`, `visualize_centrality`, `visualize_entity_types`, `visualize_relationship_matrix`) previously called `graph.get("entities", [])`, silently producing no output when passed a non-dict object. Added `_normalize_graph()` which duck-types the input — dicts pass through unchanged; any object exposing `.entities` / `.relationships` attributes (e.g. the result of `GraphBuilder.build()`) is converted to the canonical dict form; anything else raises a clear `ProcessingError` naming the offending type. 21 tests added in `tests/visualization/test_kg_visualizer_normalize_graph.py`. - **Security: 12 vulnerability fixes across CRITICAL → LOW severity** (PR `security-enhancement` by @KaifAhmad1): diff --git a/semantica/explorer/routes/graph.py b/semantica/explorer/routes/graph.py index 13ed49e9..4fe60b11 100644 --- a/semantica/explorer/routes/graph.py +++ b/semantica/explorer/routes/graph.py @@ -146,6 +146,7 @@ async def find_path( node_id: str, target: str = Query(..., description="Target node ID"), algorithm: _PathAlgorithm = Query(_PathAlgorithm.bfs, description="Algorithm: bfs or dijkstra"), + directed: bool = Query(True, description="If false, treat edges as undirected for traversal"), session: GraphSession = Depends(get_session), ): path_finder = session.path_finder @@ -159,11 +160,14 @@ async def find_path( else path_finder.bfs_shortest_path ) try: - result = await asyncio.to_thread(path_fn, graph_dict, node_id, target) + result = await asyncio.to_thread(path_fn, graph_dict, node_id, target, directed=directed) except Exception as exc: raise HTTPException(status_code=404, detail=f"No path found from '{node_id}' to '{target}': {exc}") path_nodes = result.get("path", []) if isinstance(result, dict) else (result or []) + if not path_nodes: + raise HTTPException(status_code=404, detail=f"No path found from '{node_id}' to '{target}'") + total_weight = result.get("total_weight", 0.0) if isinstance(result, dict) else 0.0 edge_ids = await asyncio.to_thread(session.resolve_path_edge_ids, path_nodes) @@ -174,6 +178,7 @@ async def find_path( path=path_nodes, edge_ids=edge_ids, total_weight=total_weight, + directed=directed, ) diff --git a/semantica/explorer/schemas.py b/semantica/explorer/schemas.py index 9bdfbbca..5b1f9778 100644 --- a/semantica/explorer/schemas.py +++ b/semantica/explorer/schemas.py @@ -67,6 +67,7 @@ class PathResponse(BaseModel): path: List[str] edge_ids: List[str] = Field(default_factory=list) total_weight: float = 0.0 + directed: bool = True class GraphStatsResponse(BaseModel): diff --git a/semantica/kg/path_finder.py b/semantica/kg/path_finder.py index d8a3e2cc..86269bfb 100644 --- a/semantica/kg/path_finder.py +++ b/semantica/kg/path_finder.py @@ -104,7 +104,8 @@ class PathFinder: source: str, target: str, weight_attribute: str = "weight", - default_weight: float = 1.0 + default_weight: float = 1.0, + directed: bool = True ) -> List[str]: """ Find shortest path using Dijkstra's algorithm. @@ -125,32 +126,34 @@ class PathFinder: """ try: self.logger.info(f"Finding Dijkstra shortest path from {source} to {target}") - + # Validate nodes exist if not self._node_exists(graph, source): raise ValueError(f"Source node {source} not found") if not self._node_exists(graph, target): raise ValueError(f"Target node {target} not found") - + + traversal_graph = graph if directed else self._make_undirected_view(graph) + # Dijkstra's algorithm distances = {source: 0.0} previous = {} priority_queue = [(0.0, source)] visited = set() - + while priority_queue: current_distance, current_node = heapq.heappop(priority_queue) - + if current_node in visited: continue - + visited.add(current_node) - + if current_node == target: break - + # Explore neighbors - for neighbor, edge_data in self._get_neighbors(graph, current_node): + for neighbor, edge_data in self._get_neighbors(traversal_graph, current_node): if neighbor in visited: continue @@ -350,44 +353,48 @@ class PathFinder: self, graph: Any, source: str, - target: str + target: str, + directed: bool = True ) -> List[str]: """ Find shortest path using BFS (unweighted). - + Args: graph: Graph object (NetworkX or similar) source: Source node ID target: Target node ID - + directed: If False, treat the graph as undirected for traversal + Returns: List of node IDs representing the shortest path - + Raises: ValueError: If source or target not found """ try: self.logger.info(f"Finding BFS shortest path from {source} to {target}") - + # Validate nodes exist if not self._node_exists(graph, source): raise ValueError(f"Source node {source} not found") if not self._node_exists(graph, target): raise ValueError(f"Target node {target} not found") - + + traversal_graph = graph if directed else self._make_undirected_view(graph) + # BFS algorithm queue = deque([(source, [source])]) visited = {source} - + while queue: current, path = queue.popleft() - + if current == target: self.logger.info(f"Found BFS path of length {len(path)}") return path - + # Explore neighbors - for neighbor, _ in self._get_neighbors(graph, current): + for neighbor, _ in self._get_neighbors(traversal_graph, current): if neighbor not in visited: visited.add(neighbor) queue.append((neighbor, path + [neighbor])) @@ -564,6 +571,18 @@ class PathFinder: return False return False + def _make_undirected_view(self, graph: Any) -> Any: + """Return an undirected view of the graph for bidirectional traversal. + + For NetworkX directed graphs this calls ``to_undirected()``, which + preserves all edge attributes. For graph types that have no such + method the original object is returned as a fallback — callers that + already expose undirected neighbors will still work correctly. + """ + if hasattr(graph, "to_undirected"): + return graph.to_undirected() + return graph + def _get_neighbors(self, graph: Any, node: str) -> List[Tuple[str, Any]]: """Get neighbors of a node with edge data.""" neighbors = [] diff --git a/tests/explorer/test_explorer_api.py b/tests/explorer/test_explorer_api.py index 3213db23..0be0c783 100644 --- a/tests/explorer/test_explorer_api.py +++ b/tests/explorer/test_explorer_api.py @@ -4,6 +4,7 @@ import json from pathlib import Path import uuid +import networkx as nx import pytest from semantica.context.context_graph import ContextGraph @@ -638,3 +639,122 @@ class TestGenericGraphFileLoading: assert repeat.status_code == 200 repeat_ids = [edge["id"] for edge in repeat.json()["edges"]] assert repeat_ids == ["edge-alpha", "edge-beta"] + + +# --------------------------------------------------------------------------- +# Bidirectional path-finding tests (issue #469) +# --------------------------------------------------------------------------- + +def _make_path_session() -> GraphSession: + """Return a GraphSession whose build_graph_dict yields an nx.DiGraph with A→B only. + + GraphSession wraps a ContextGraph (required by create_app), but we patch + build_graph_dict so PathFinder receives an actual NetworkX DiGraph — the + graph type the Explorer is designed to traverse for path queries. + """ + cg = ContextGraph(advanced_analytics=False) + cg.add_node("A", node_type="entity", content="Node A") + cg.add_node("B", node_type="entity", content="Node B") + cg.add_edge("A", "B", edge_type="connects") + + session = GraphSession(cg) + + # Patch build_graph_dict to return the directed NetworkX graph that + # PathFinder needs. The ContextGraph dict format is not traversable by + # PathFinder; this mimics how a KG-backed session would expose the graph. + digraph = nx.DiGraph() + digraph.add_edge("A", "B") + session.build_graph_dict = lambda node_ids=None: digraph # type: ignore[method-assign] + + return session + + +@pytest.fixture +def path_client(): + session = _make_path_session() + app = create_app(session=session) + with TestClient(app) as c: + yield c + + +class TestBidirectionalPathRoute: + """API-level tests for directed=true/false on GET /api/graph/node/{id}/path.""" + + # ------------------------------------------------------------------ + # directed=true (default) — existing directed-only behaviour + # ------------------------------------------------------------------ + + def test_directed_true_forward_path_found(self, path_client): + """A→B exists: forward query with directed=true must succeed.""" + resp = path_client.get("/api/graph/node/A/path?target=B&directed=true") + assert resp.status_code == 200 + body = resp.json() + assert body["path"] == ["A", "B"] + assert body["directed"] is True + + def test_directed_true_reverse_returns_404(self, path_client): + """Only A→B exists: reverse query with directed=true must return 404.""" + resp = path_client.get("/api/graph/node/B/path?target=A&directed=true") + assert resp.status_code == 404 + + def test_default_param_reverse_returns_404(self, path_client): + """Omitting directed= must preserve current directed behaviour (404 for reverse).""" + resp = path_client.get("/api/graph/node/B/path?target=A") + assert resp.status_code == 404 + + # ------------------------------------------------------------------ + # directed=false — new undirected traversal + # ------------------------------------------------------------------ + + def test_directed_false_reverse_path_found(self, path_client): + """directed=false must find B→A even though only A→B exists.""" + resp = path_client.get("/api/graph/node/B/path?target=A&directed=false") + assert resp.status_code == 200 + body = resp.json() + assert body["path"] == ["B", "A"] + assert body["directed"] is False + + def test_directed_false_forward_path_found(self, path_client): + """directed=false must not break the natural A→B direction.""" + resp = path_client.get("/api/graph/node/A/path?target=B&directed=false") + assert resp.status_code == 200 + body = resp.json() + assert body["path"] == ["A", "B"] + assert body["directed"] is False + + # ------------------------------------------------------------------ + # Algorithm variants + # ------------------------------------------------------------------ + + def test_dijkstra_directed_false_reverse(self, path_client): + resp = path_client.get( + "/api/graph/node/B/path?target=A&algorithm=dijkstra&directed=false" + ) + assert resp.status_code == 200 + body = resp.json() + assert body["path"] == ["B", "A"] + assert body["algorithm"] == "dijkstra" + assert body["directed"] is False + + def test_dijkstra_directed_true_reverse_returns_404(self, path_client): + resp = path_client.get( + "/api/graph/node/B/path?target=A&algorithm=dijkstra&directed=true" + ) + assert resp.status_code == 404 + + # ------------------------------------------------------------------ + # PathResponse schema + # ------------------------------------------------------------------ + + def test_response_schema_includes_directed_field(self, path_client): + """PathResponse must always include the directed field.""" + resp = path_client.get("/api/graph/node/A/path?target=B") + assert resp.status_code == 200 + body = resp.json() + assert "directed" in body + + def test_response_directed_reflects_query_param(self, path_client): + resp_true = path_client.get("/api/graph/node/A/path?target=B&directed=true") + resp_false = path_client.get("/api/graph/node/A/path?target=B&directed=false") + assert resp_true.json()["directed"] is True + assert resp_false.json()["directed"] is False diff --git a/tests/kg/test_path_finder.py b/tests/kg/test_path_finder.py index 7c76abbd..a4fab78a 100644 --- a/tests/kg/test_path_finder.py +++ b/tests/kg/test_path_finder.py @@ -821,3 +821,87 @@ class TestPathFinderEdgeCases: paths = self.finder.all_shortest_paths(single_node_graph, "A") assert len(paths) == 0 # No paths to other nodes + + +class TestBidirectionalPathFinding: + """Tests for the directed=False undirected-traversal mode (issue #469).""" + + def setup_method(self): + self.finder = PathFinder() + # Single directed edge A → B. Reverse query B → A has no directed path. + self.digraph = nx.DiGraph() + self.digraph.add_edge("A", "B") + + # --- directed=True (default) preserves existing behaviour --- + + def test_bfs_directed_true_reverse_returns_empty(self): + """B→A should find nothing when directed=True (default).""" + path = self.finder.bfs_shortest_path(self.digraph, "B", "A", directed=True) + assert path == [] + + def test_dijkstra_directed_true_reverse_returns_empty(self): + """B→A should find nothing when directed=True (default).""" + path = self.finder.dijkstra_shortest_path(self.digraph, "B", "A", directed=True) + assert path == [] + + def test_bfs_directed_true_default_arg(self): + """Omitting directed= should behave the same as directed=True.""" + path = self.finder.bfs_shortest_path(self.digraph, "B", "A") + assert path == [] + + def test_dijkstra_directed_true_default_arg(self): + path = self.finder.dijkstra_shortest_path(self.digraph, "B", "A") + assert path == [] + + # --- directed=False finds path against edge orientation --- + + def test_bfs_directed_false_reverse_single_edge(self): + """directed=False must find B→A even though only A→B exists.""" + path = self.finder.bfs_shortest_path(self.digraph, "B", "A", directed=False) + assert path == ["B", "A"] + + def test_dijkstra_directed_false_reverse_single_edge(self): + path = self.finder.dijkstra_shortest_path(self.digraph, "B", "A", directed=False) + assert path == ["B", "A"] + + def test_bfs_directed_false_forward_still_works(self): + """directed=False should not break the forward direction.""" + path = self.finder.bfs_shortest_path(self.digraph, "A", "B", directed=False) + assert path == ["A", "B"] + + def test_dijkstra_directed_false_forward_still_works(self): + path = self.finder.dijkstra_shortest_path(self.digraph, "A", "B", directed=False) + assert path == ["A", "B"] + + # --- multi-hop path where one edge is against the query direction --- + + def test_bfs_directed_false_multihop(self): + """A→B, C→B graph: directed=False lets us find A→B→C (i.e. A→C via B).""" + g = nx.DiGraph() + g.add_edge("A", "B") + g.add_edge("C", "B") # oriented towards B, not away from it + # undirected view: A-B-C, so A→C path exists + path = self.finder.bfs_shortest_path(g, "A", "C", directed=False) + assert path[0] == "A" and path[-1] == "C" + assert "B" in path + + def test_dijkstra_directed_false_multihop(self): + g = nx.DiGraph() + g.add_edge("A", "B") + g.add_edge("C", "B") + path = self.finder.dijkstra_shortest_path(g, "A", "C", directed=False) + assert path[0] == "A" and path[-1] == "C" + assert "B" in path + + # --- PathResponse.directed field --- + + def test_path_response_directed_field_exists(self): + """PathResponse must carry a directed field.""" + from semantica.explorer.schemas import PathResponse + r = PathResponse(source="A", target="B", algorithm="bfs", path=["A", "B"], directed=False) + assert r.directed is False + + def test_path_response_directed_field_defaults_true(self): + from semantica.explorer.schemas import PathResponse + r = PathResponse(source="A", target="B", algorithm="bfs", path=["A", "B"]) + assert r.directed is True