Fix two review findings in SKOS cycle validation

- validate_skos_hierarchy() re-walked every existing hierarchy edge in
  the graph on each write, so one pre-existing cycle anywhere would
  block all unrelated future SKOS writes. It now only traverses
  concepts touched by the edges actually being written, while still
  checking those against existing edges for cross-boundary cycles.
- In /api/ontology/load, `except HTTPException: raise` sat after a
  broader `except Exception` clause that already matched HTTPException,
  so a 422 raised after a successful OntologyIngestor parse was
  silently swallowed and retried via the fallback RDF parser instead of
  reaching the caller. Reordered the except clauses.

Co-authored-by: mikemikimike <13286568797@163.com>

CHANGELOG.md

@@ -47,6 +47,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - Added cycle detection (`validate_skos_hierarchy`) for `skos:broader` and `skos:narrower` relationships in `ContextGraph.add_edge()` and `ContextGraph.add_edges()`, preventing direct 2-node cycles, self-loops, and multi-hop hierarchy cycles
   - Added `GraphSession.add_nodes_and_edges()` to validate SKOS hierarchy edges upfront under lock before node insertion, preventing partial-write leaks where nodes remain after a cyclic edge is rejected
   - Updated vocabulary, ontology (`/api/ontology/load`, `/api/ontology/create`), and JSON/CSV import routes to use `add_nodes_and_edges()` and return HTTP 422 with actionable error messages when a cycle is detected
+  - Follow-up fix by @KaifAhmad1: `validate_skos_hierarchy()` previously re-walked *every* SKOS hierarchy edge already in the graph on each write, so one pre-existing cycle anywhere (e.g. legacy data) blocked all unrelated future writes; it now only traverses concepts touched by the edges being written, while still checking against existing edges for cycles that span old and new data
+  - Follow-up fix by @KaifAhmad1: in `/api/ontology/load`, `except HTTPException: raise` was unreachable because a broader `except Exception` clause above it already matched `HTTPException`, so a 422 raised after a successful `OntologyIngestor` parse was silently swallowed and reprocessed via the fallback RDF parser; reordered the clauses so the deliberate 422 always propagates
 
 - **`AgnoDecisionKit`/`AgnoKGToolkit` silently swallowed Agno tool registration failures** (#780, #818) by @Sameer6305 and @KaifAhmad1
   - Removed the `try/except: pass` wrapped around `self.register(fn)` in both toolkits' `__init__`; when Agno is installed, a registration failure now propagates immediately instead of leaving the toolkit half-registered with no signal to the caller

semantica/context/context_graph.py

@@ -597,7 +597,7 @@ class ContextGraph:
                 existing_edges = [
                     edge for edge in self.find_edges() if is_skos_hierarchy_edge(edge)
                 ]
-                validate_skos_hierarchy([*existing_edges, *hierarchy_edges])
+                validate_skos_hierarchy(hierarchy_edges, existing_edges)
             for raw_edge in edges:
                 if not isinstance(raw_edge, dict):
                     continue
@@ -962,7 +962,7 @@ class ContextGraph:
                 existing_edges = [
                     edge for edge in self.find_edges() if is_skos_hierarchy_edge(edge)
                 ]
-                validate_skos_hierarchy([*existing_edges, candidate])
+                validate_skos_hierarchy([candidate], existing_edges)
             return self._add_internal_edge(
                 ContextEdge(
                     edge_id=edge_id,

semantica/explorer/routes/ontology.py

@@ -1339,15 +1339,19 @@ async def load_ontology(
             except OSError as cleanup_exc:
                 logger.debug("Failed to remove temporary ontology file: %s", cleanup_exc)
                 
+    except HTTPException:
+        # Re-raise HTTPExceptions we deliberately raised above (e.g. the 422 from
+        # SKOS cycle validation) instead of letting the broad `except Exception`
+        # below mask them as an ingestor failure and silently retry via the
+        # fallback parser.
+        raise
     except Exception as ingest_exc:
         logger.warning(f"OntologyIngestor failed, falling back to basic parsing: {ingest_exc}")
-        
+
         # Fallback to basic parsing
         nodes, edges, metadata = await asyncio.to_thread(
             _parse_rdf_sync, content_str.encode("utf-8"), fmt
         )
-    except HTTPException:
-        raise
     except Exception as exc:
         raise HTTPException(status_code=422, detail=f"Could not parse ontology: {exc}") from exc
 

semantica/explorer/session.py

@@ -754,7 +754,7 @@ class GraphSession:
         existing_edges = [
             edge for edge in self.graph.find_edges() if is_skos_hierarchy_edge(edge)
         ]
-        validate_skos_hierarchy([*existing_edges, *hierarchy_edges])
+        validate_skos_hierarchy(hierarchy_edges, existing_edges)
 
     def add_nodes_and_edges(
         self,

semantica/utils/skos.py

@@ -17,41 +17,65 @@ def is_skos_hierarchy_edge(edge: Mapping[str, object]) -> bool:
     )
 
 
-def validate_skos_hierarchy(edges: Iterable[Mapping[str, object]]) -> None:
-    """Raise ``ValueError`` when SKOS hierarchy edges contain a cycle.
+def _child_parent(edge: Mapping[str, object]) -> tuple[str, str] | None:
+    """Normalize a SKOS hierarchy edge to a ``(child, parent)`` pair, or ``None``."""
+    if not isinstance(edge, Mapping) or not is_skos_hierarchy_edge(edge):
+        return None
+    edge_type = next(
+        (
+            edge.get(key)
+            for key in ("type", "edge_type", "relationship", "predicate", "relation")
+            if edge.get(key) in _HIERARCHY_EDGE_TYPES
+        ),
+        None,
+    )
+    if edge_type not in _HIERARCHY_EDGE_TYPES:
+        return None
+
+    raw_source = edge.get("source", edge.get("source_id"))
+    raw_target = edge.get("target", edge.get("target_id"))
+    if raw_source is None or raw_target is None:
+        return None
+    source = str(raw_source).strip()
+    target = str(raw_target).strip()
+    if not source or not target:
+        return None
+
+    return (source, target) if edge_type == "skos:broader" else (target, source)
+
+
+def validate_skos_hierarchy(
+    new_edges: Iterable[Mapping[str, object]],
+    existing_edges: Iterable[Mapping[str, object]] = (),
+) -> None:
+    """Raise ``ValueError`` when adding ``new_edges`` would introduce a cycle.
 
     ``skos:broader`` points from a concept to its parent while
     ``skos:narrower`` expresses the same relationship in the opposite
     direction. Both forms are normalized to child-to-parent adjacency before
     cycle detection.
+
+    ``existing_edges`` supplies the SKOS hierarchy edges already persisted in
+    the graph so that cycles spanning old and new edges are still caught.
+    Only the concepts touched by ``new_edges`` are checked, though: a cycle
+    that already exists entirely within ``existing_edges`` must not block an
+    unrelated write elsewhere in the graph.
     """
-
     parents: dict[str, set[str]] = defaultdict(set)
-    for edge in edges:
-        if not isinstance(edge, Mapping) or not is_skos_hierarchy_edge(edge):
-            continue
-        edge_type = next(
-            (
-                edge.get(key)
-                for key in ("type", "edge_type", "relationship", "predicate", "relation")
-                if edge.get(key) in _HIERARCHY_EDGE_TYPES
-            ),
-            None,
-        )
-        if edge_type not in _HIERARCHY_EDGE_TYPES:
-            continue
+    for edge in existing_edges:
+        pair = _child_parent(edge)
+        if pair is not None:
+            parents[pair[0]].add(pair[1])
 
-        raw_source = edge.get("source", edge.get("source_id"))
-        raw_target = edge.get("target", edge.get("target_id"))
-        if raw_source is None or raw_target is None:
+    touched: set[str] = set()
+    for edge in new_edges:
+        pair = _child_parent(edge)
+        if pair is None:
             continue
-        source = str(raw_source).strip()
-        target = str(raw_target).strip()
-        if not source or not target:
-            continue
-
-        child, parent = ((source, target) if edge_type == "skos:broader" else (target, source))
+        child, parent = pair
         parents[child].add(parent)
+        touched.add(child)
+        touched.add(parent)
 
     visiting: set[str] = set()
     visited: set[str] = set()
@@ -68,5 +92,5 @@ def validate_skos_hierarchy(edges: Iterable[Mapping[str, object]]) -> None:
         visiting.remove(concept)
         visited.add(concept)
 
-    for concept in parents:
+    for concept in touched:
         visit(concept)

tests/context/test_context.py

@@ -223,6 +223,19 @@ class TestContextModule(unittest.TestCase):
 
         self.assertEqual(len(graph.edges), 0)
 
+    def test_context_graph_preexisting_unrelated_cycle_does_not_block_new_write(self):
+        """A cycle already persisted elsewhere in the graph (e.g. legacy data
+        written before cycle detection existed) must not poison unrelated
+        SKOS hierarchy writes for concepts it doesn't touch."""
+        from semantica.context.context_graph import ContextEdge
+
+        graph = ContextGraph()
+        graph._add_internal_edge(ContextEdge(source_id="X", target_id="Y", edge_type="skos:broader"))
+        graph._add_internal_edge(ContextEdge(source_id="Y", target_id="X", edge_type="skos:broader"))
+
+        self.assertTrue(graph.add_edge("C", "D", "skos:broader"))
+        self.assertEqual(len(graph.edges), 3)
+
     # --- AgentContext Tests ---
     @patch('semantica.context.agent_memory.AgentMemory._generate_embedding')
     def test_agent_context_end_to_end(self, mock_gen_embedding):

tests/explorer/test_ontology_subissue3.py

@@ -658,3 +658,40 @@ ex:B a skos:Concept ; skos:prefLabel "Beta" ; skos:inScheme ex:S ; skos:broader
     assert "cycle" in response.json()["detail"].lower()
 
 
+def test_ontology_load_does_not_swallow_422_from_ingestor_success_path(client):
+    """A ValueError raised by add_nodes_and_edges() after OntologyIngestor
+    succeeds must surface as its own 422, not be masked by the broad
+    `except Exception` fallback-to-basic-parsing handler and silently
+    retried under a different parser."""
+    from semantica.ingest.ontology_ingestor import OntologyData
+
+    fake_data = OntologyData(
+        data={
+            "uri": "http://example.org/onto-fake",
+            "name": "Fake Ontology",
+            "classes": [{"uri": "http://example.org/onto-fake#A", "name": "A"}],
+            "properties": [],
+        },
+        source_path="fake.ttl",
+        format="turtle",
+    )
+
+    with patch(
+        "semantica.ingest.ontology_ingestor.OntologyIngestor.ingest_ontology",
+        return_value=fake_data,
+    ), patch(
+        "semantica.explorer.session.GraphSession.add_nodes_and_edges",
+        side_effect=ValueError("SKOS hierarchy contains a cycle involving 'A'."),
+    ), patch(
+        "semantica.explorer.routes.ontology._parse_rdf_sync"
+    ) as fallback_parse:
+        response = client.post(
+            "/api/ontology/load",
+            json={"content": "@prefix ex: <http://example.org/> . ex:A a ex:Thing .", "format": "turtle"},
+        )
+
+    assert response.status_code == 422
+    assert "cycle" in response.json()["detail"].lower()
+    fallback_parse.assert_not_called()
+
+

tests/utils/test_skos.py

@@ -0,0 +1,64 @@
+import unittest
+
+from semantica.utils.skos import is_skos_hierarchy_edge, validate_skos_hierarchy
+
+
+class TestIsSkosHierarchyEdge(unittest.TestCase):
+
+    def test_recognizes_broader_and_narrower(self):
+        self.assertTrue(is_skos_hierarchy_edge({"source": "A", "target": "B", "type": "skos:broader"}))
+        self.assertTrue(is_skos_hierarchy_edge({"source": "A", "target": "B", "type": "skos:narrower"}))
+
+    def test_ignores_other_edge_types(self):
+        self.assertFalse(is_skos_hierarchy_edge({"source": "A", "target": "B", "type": "rdfs:subClassOf"}))
+
+    def test_ignores_non_mapping(self):
+        self.assertFalse(is_skos_hierarchy_edge("not-a-dict"))
+
+
+class TestValidateSkosHierarchy(unittest.TestCase):
+
+    def test_accepts_acyclic_chain(self):
+        validate_skos_hierarchy([
+            {"source": "A", "target": "B", "type": "skos:broader"},
+            {"source": "B", "target": "C", "type": "skos:broader"},
+        ])
+
+    def test_rejects_self_loop(self):
+        with self.assertRaisesRegex(ValueError, "cycle"):
+            validate_skos_hierarchy([{"source": "A", "target": "A", "type": "skos:broader"}])
+
+    def test_rejects_direct_two_node_cycle(self):
+        with self.assertRaisesRegex(ValueError, "cycle"):
+            validate_skos_hierarchy([
+                {"source": "A", "target": "B", "type": "skos:broader"},
+                {"source": "B", "target": "A", "type": "skos:broader"},
+            ])
+
+    def test_rejects_cycle_spanning_existing_and_new_edges(self):
+        existing = [
+            {"source": "A", "target": "B", "type": "skos:broader"},
+            {"source": "B", "target": "C", "type": "skos:broader"},
+        ]
+        with self.assertRaisesRegex(ValueError, "cycle"):
+            validate_skos_hierarchy([{"source": "C", "target": "A", "type": "skos:broader"}], existing)
+
+    def test_preexisting_unrelated_cycle_does_not_block_new_write(self):
+        """A cycle already persisted elsewhere in the graph (e.g. legacy data
+        written before cycle detection existed) must not poison unrelated
+        writes for concepts it doesn't touch."""
+        existing = [
+            {"source": "X", "target": "Y", "type": "skos:broader"},
+            {"source": "Y", "target": "X", "type": "skos:broader"},
+        ]
+        validate_skos_hierarchy([{"source": "C", "target": "D", "type": "skos:broader"}], existing)
+
+    def test_none_and_blank_endpoints_are_ignored(self):
+        validate_skos_hierarchy([
+            {"source": None, "target": "B", "type": "skos:broader"},
+            {"source": "  ", "target": "B", "type": "skos:broader"},
+        ])
+
+
+if __name__ == "__main__":
+    unittest.main()