Merge pull request #823 from Sameer6305/fix/775-ontology-atomic-writes

fix(explorer): complete atomic ontology refresh writes - #775

CHANGELOG.md

@@ -59,6 +59,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - 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
+  - Follow-up fix (#775): `/api/ontology/{uri}/refresh` was missed by the original sweep and still called `session.add_nodes()` then `session.add_edges()` as two independent operations, so a cyclic SKOS edge rejected by `add_edges()` left the nodes from the preceding `add_nodes()` call committed to the graph; switched to `session.add_nodes_and_edges()` with the same `except ValueError` → HTTP 422 handling already used by `/api/ontology/load` and `/api/ontology/create`. Audited every other `add_nodes()`/`add_edges()` pairing in the repo (`GraphStore`, `graph_builder.py`, `agent_memory.py`, `context_graph.py.load()`, `enrich.py`) — none share `GraphSession`'s SKOS-cycle-validation write path, so none were changed
 
 - **Agno `_AgentScopedStore.upsert_memory` silently swallowed decision recording failures** (#779)
   - `upsert_memory()` now logs `logger.warning("[%s] record_decision failed: %s", self._role, exc, exc_info=True)` when `record_decision()` fails, matching the error-logging convention used for `store()` in the same method with traceback context preserved

semantica/explorer/routes/ontology.py

@@ -2770,8 +2770,13 @@ async def refresh_ontology(
     except Exception as exc:
         raise HTTPException(status_code=422, detail=f"Refresh parse error: {exc}") from exc
 
-    nodes_added = await asyncio.to_thread(session.add_nodes, nodes)
-    edges_added = await asyncio.to_thread(session.add_edges, edges)
+    try:
+        nodes_added, edges_added = await asyncio.to_thread(
+            session.add_nodes_and_edges, nodes, edges
+        )
+    except ValueError as exc:
+        raise HTTPException(status_code=422, detail=str(exc)) from exc
+
     entry.loaded_at = datetime.now(UTC).isoformat()
 
     return RefreshResponse(uri=ontology_uri, nodes_added=nodes_added, edges_added=edges_added)

tests/explorer/test_ontology_subissue3.py

@@ -1,11 +1,13 @@
 """Tests for Ontology Hub subissue 3 APIs."""
 
 from unittest.mock import patch
+from urllib.parse import quote
 
 import pytest
 
 from semantica.context.context_graph import ContextGraph
 from semantica.explorer.app import create_app
+from semantica.explorer.routes.ontology import OntologyEntry
 from semantica.explorer.session import GraphSession
 
 try:
@@ -695,3 +697,98 @@ def test_ontology_load_does_not_swallow_422_from_ingestor_success_path(client):
     fallback_parse.assert_not_called()
 
 
+# ---------------------------------------------------------------------------
+# refresh_ontology — single combined add_nodes_and_edges() coverage (#775)
+# ---------------------------------------------------------------------------
+
+_REFRESH_TTL = """
+@prefix owl: <http://www.w3.org/2002/07/owl#> .
+@prefix rdfs: <http://www.w3.org/2000/01/rdf-schema#> .
+@prefix ex: <http://example.org/refresh-onto#> .
+<http://example.org/refresh-onto> a owl:Ontology .
+ex:Widget a owl:Class ; rdfs:label "Widget" .
+ex:Gadget a owl:Class ; rdfs:label "Gadget" ; rdfs:subClassOf ex:Widget .
+"""
+
+_REFRESH_CYCLIC_TTL = """
+@prefix skos: <http://www.w3.org/2004/02/skos/core#> .
+@prefix ex:   <http://example.org/refresh-onto#> .
+ex:S a skos:ConceptScheme ; skos:prefLabel "Scheme" .
+ex:A a skos:Concept ; skos:prefLabel "Alpha" ; skos:inScheme ex:S ; skos:broader ex:B .
+ex:B a skos:Concept ; skos:prefLabel "Beta" ; skos:inScheme ex:S ; skos:broader ex:A .
+"""
+
+
+def _register_refresh_entry(client, uri="http://example.org/refresh-onto", source_url="http://example.org/refresh-onto.ttl"):
+    entry = OntologyEntry(
+        uri=uri,
+        name="Refresh Ontology",
+        format="turtle",
+        status="external",
+        source_url=source_url,
+        loaded_at="2024-01-01T00:00:00+00:00",
+    )
+    # Registry is created lazily on app.state by _get_registry; seed it directly.
+    if not hasattr(client.app.state, "ontology_registry"):
+        client.app.state.ontology_registry = {}
+    client.app.state.ontology_registry[uri] = entry
+    return entry
+
+
+def test_refresh_ontology_success_adds_nodes_and_edges(client):
+    entry = _register_refresh_entry(client)
+    encoded_uri = quote(entry.uri, safe="")
+
+    with patch(
+        "semantica.explorer.routes.ontology._fetch_url_sync",
+        return_value=_REFRESH_TTL.encode("utf-8"),
+    ):
+        response = client.post(f"/api/ontology/{encoded_uri}/refresh")
+
+    assert response.status_code == 200
+    payload = response.json()
+    assert payload["uri"] == entry.uri
+    assert payload["nodes_added"] >= 2   # Widget, Gadget (+ ontology node depending on parser)
+    assert payload["edges_added"] >= 1   # Gadget rdfs:subClassOf Widget
+
+
+def test_refresh_ontology_rejects_cyclic_skos_hierarchy_without_partial_write(client):
+    entry = _register_refresh_entry(client)
+    encoded_uri = quote(entry.uri, safe="")
+
+    graph = client.app.state.session.graph
+    nodes_before = len(graph.nodes)
+
+    with patch(
+        "semantica.explorer.routes.ontology._fetch_url_sync",
+        return_value=_REFRESH_CYCLIC_TTL.encode("utf-8"),
+    ), patch(
+        "semantica.explorer.session.GraphSession.add_nodes_and_edges",
+        wraps=client.app.state.session.add_nodes_and_edges,
+    ) as spy:
+        response = client.post(f"/api/ontology/{encoded_uri}/refresh")
+
+    assert response.status_code == 422
+    assert "cycle" in response.json()["detail"].lower()
+    # A single combined add_nodes_and_edges() call was made — not separate
+    # add_nodes()/add_edges() calls — so the upfront SKOS validation runs
+    # before either write and the cyclic edges leave no nodes behind in the
+    # graph. add_nodes_and_edges() provides pre-write validation and
+    # lock-based mutual exclusion, not general transactional rollback.
+    spy.assert_called_once()
+    assert len(graph.nodes) == nodes_before
+
+
+def test_refresh_ontology_unknown_uri_returns_404(client):
+    response = client.post("/api/ontology/http%3A%2F%2Fexample.org%2Fnot-registered/refresh")
+    assert response.status_code == 404
+
+
+def test_refresh_ontology_missing_source_url_returns_422(client):
+    entry = _register_refresh_entry(client, uri="http://example.org/refresh-onto-nosrc", source_url=None)
+    encoded_uri = quote(entry.uri, safe="")
+    response = client.post(f"/api/ontology/{encoded_uri}/refresh")
+    assert response.status_code == 422
+    assert "source url" in response.json()["detail"].lower()
+
+