From d293ca6009e7f362866fbb52b1aae8e89de47b5d Mon Sep 17 00:00:00 2001 From: Sameer6305 Date: Sat, 1 Aug 2026 19:12:00 +0530 Subject: [PATCH 1/2] fix(explorer): complete atomic ontology refresh writes (#775) --- CHANGELOG.md | 1 + semantica/explorer/routes/ontology.py | 9 ++- tests/explorer/test_ontology_subissue3.py | 91 +++++++++++++++++++++++ 3 files changed, 99 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9649c8b..b8852020 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,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 - **`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 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..f223f810 100644 --- a/tests/explorer/test_ontology_subissue3.py +++ b/tests/explorer/test_ontology_subissue3.py @@ -6,7 +6,9 @@ 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 +from semantica.utils.skos import validate_skos_hierarchy try: from starlette.testclient import TestClient @@ -695,3 +697,92 @@ def test_ontology_load_does_not_swallow_422_from_ingestor_success_path(client): fallback_parse.assert_not_called() +# --------------------------------------------------------------------------- +# refresh_ontology — atomic 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) + + with patch( + "semantica.explorer.routes.ontology._fetch_url_sync", + return_value=_REFRESH_TTL.encode("utf-8"), + ): + response = client.post(f"/api/ontology/{entry.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) + + 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/{entry.uri}/refresh") + + assert response.status_code == 422 + assert "cycle" in response.json()["detail"].lower() + # A single atomic call was made — not separate add_nodes()/add_edges() calls — + # and the cyclic edges must not have left any nodes behind in the graph. + 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) + response = client.post(f"/api/ontology/{entry.uri}/refresh") + assert response.status_code == 422 + assert "source url" in response.json()["detail"].lower() + + From 6f6c825f3d2efba668e11794c527ab4bcb2cca0e Mon Sep 17 00:00:00 2001 From: Sameer6305 Date: Sat, 1 Aug 2026 19:36:16 +0530 Subject: [PATCH 2/2] fixed qodo reviews - Removed unused `validate_skos_hierarchy` import from test_ontology_subissue3.py (flake8 F401); the test uses a `wraps=` spy on the real add_nodes_and_edges instead of calling the helper directly. - refresh_ontology tests now percent-encode the ontology URI with urllib.parse.quote before interpolating it into the {ontology_uri:path} request path, matching the already-encoded unknown-uri refresh test in the same file instead of embedding a raw http://... URI with slashes. - Reworded the cyclic-SKOS refresh test's comment and section header: GraphSession.add_nodes_and_edges() documents pre-write validation and lock-based mutual exclusion, not transactional rollback, so "atomic" was replaced with "single combined add_nodes_and_edges() call" to avoid implying rollback guarantees that don't exist. Verified: tests/explorer/test_ontology_subissue3.py (34 passed) and tests/explorer/ (204 passed), no regressions. --- tests/explorer/test_ontology_subissue3.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/tests/explorer/test_ontology_subissue3.py b/tests/explorer/test_ontology_subissue3.py index f223f810..56800c50 100644 --- a/tests/explorer/test_ontology_subissue3.py +++ b/tests/explorer/test_ontology_subissue3.py @@ -1,6 +1,7 @@ """Tests for Ontology Hub subissue 3 APIs.""" from unittest.mock import patch +from urllib.parse import quote import pytest @@ -8,7 +9,6 @@ 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 -from semantica.utils.skos import validate_skos_hierarchy try: from starlette.testclient import TestClient @@ -698,7 +698,7 @@ def test_ontology_load_does_not_swallow_422_from_ingestor_success_path(client): # --------------------------------------------------------------------------- -# refresh_ontology — atomic add_nodes_and_edges() coverage (#775) +# refresh_ontology — single combined add_nodes_and_edges() coverage (#775) # --------------------------------------------------------------------------- _REFRESH_TTL = """ @@ -737,12 +737,13 @@ def _register_refresh_entry(client, uri="http://example.org/refresh-onto", sourc 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/{entry.uri}/refresh") + response = client.post(f"/api/ontology/{encoded_uri}/refresh") assert response.status_code == 200 payload = response.json() @@ -753,6 +754,7 @@ def test_refresh_ontology_success_adds_nodes_and_edges(client): 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) @@ -764,12 +766,15 @@ def test_refresh_ontology_rejects_cyclic_skos_hierarchy_without_partial_write(cl "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/{entry.uri}/refresh") + response = client.post(f"/api/ontology/{encoded_uri}/refresh") assert response.status_code == 422 assert "cycle" in response.json()["detail"].lower() - # A single atomic call was made — not separate add_nodes()/add_edges() calls — - # and the cyclic edges must not have left any nodes behind in the graph. + # 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 @@ -781,7 +786,8 @@ def test_refresh_ontology_unknown_uri_returns_404(client): 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) - response = client.post(f"/api/ontology/{entry.uri}/refresh") + 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()