mirror of
https://github.com/semantica-agi/semantica.git
synced 2026-08-29 04:26:20 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
952a4530f5
commit
523b02083f
+2
-1
@@ -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):
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -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):
|
||||
|
||||
+38
-19
@@ -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 = []
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user