diff --git a/CHANGELOG.md b/CHANGELOG.md index b03a6065..badd93d8 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/semantica/explorer/routes/ontology.py b/semantica/explorer/routes/ontology.py index a9cce469..4944f2ef 100644 --- a/semantica/explorer/routes/ontology.py +++ b/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) diff --git a/tests/explorer/test_ontology_subissue3.py b/tests/explorer/test_ontology_subissue3.py index a106d6d7..56800c50 100644 --- a/tests/explorer/test_ontology_subissue3.py +++ b/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: . +@prefix rdfs: . +@prefix ex: . + 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: . +@prefix ex: . +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() + +