From 0ed5ec00b3c3d1a2e206ac37d9bb56de302d3603 Mon Sep 17 00:00:00 2001 From: SAY-5 Date: Thu, 7 May 2026 02:47:15 -0700 Subject: [PATCH] fix(ontology): guard against empty query string in SPARQL generator Closes #870. query.split()[0] in _generate_with_llm raised IndexError when the LLM response 'query' field passed the keyword-startswith guard but split to an empty list (e.g. if the guard is ever weakened during refactoring). Compute parts once, return None when empty, and reuse parts[0] for the query_type read. Signed-off-by: SAY-5 --- .../unit/test_query/test_sparql_generator.py | 115 ++++++++++++++++++ .../query/ontology/sparql_generator.py | 5 +- 2 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_query/test_sparql_generator.py diff --git a/tests/unit/test_query/test_sparql_generator.py b/tests/unit/test_query/test_sparql_generator.py new file mode 100644 index 00000000..27dd1d94 --- /dev/null +++ b/tests/unit/test_query/test_sparql_generator.py @@ -0,0 +1,115 @@ +""" +Tests for the ontology-driven SPARQL generator. + +Covers regression for issue #870: an empty / whitespace-only LLM response +must not raise IndexError when the keyword-startswith guard is bypassed. +""" + +import pytest +from unittest.mock import AsyncMock, MagicMock + +# The trustgraph.query.ontology package __init__ chains imports that can +# fail outside the deployed FlowProcessor environment, so the suite is +# skipped at collection if the module cannot be imported. +sparql_generator = pytest.importorskip( + "trustgraph.query.ontology.sparql_generator", +) +question_analyzer = pytest.importorskip( + "trustgraph.query.ontology.question_analyzer", +) +ontology_matcher = pytest.importorskip( + "trustgraph.query.ontology.ontology_matcher", +) + +SPARQLGenerator = sparql_generator.SPARQLGenerator +SPARQLQuery = sparql_generator.SPARQLQuery +QuestionComponents = question_analyzer.QuestionComponents +QuestionType = question_analyzer.QuestionType +QueryOntologySubset = ontology_matcher.QueryOntologySubset + + +def _make_question_components(): + return QuestionComponents( + original_question="dummy", + question_type=QuestionType.RETRIEVAL, + entities=[], + relationships=[], + constraints=[], + aggregations=[], + expected_answer_type="entity", + keywords=[], + ) + + +def _make_ontology_subset(): + return QueryOntologySubset( + ontology_id="test", + classes={}, + object_properties={}, + datatype_properties={}, + metadata={"namespace": "http://example.org/"}, + ) + + +class TestGenerateWithLlm: + """Regression tests for _generate_with_llm parsing safety.""" + + @pytest.mark.asyncio + async def test_valid_select_response_returns_query(self): + """Sanity: a well-formed LLM response yields a SPARQLQuery.""" + prompt_service = MagicMock() + prompt_service.generate_sparql = AsyncMock(return_value={ + "query": "SELECT ?s WHERE { ?s ?p ?o }", + "explanation": "test", + }) + generator = SPARQLGenerator(prompt_service=prompt_service) + + result = await generator._generate_with_llm( + _make_question_components(), _make_ontology_subset(), + ) + + assert isinstance(result, SPARQLQuery) + assert result.query_type == "SELECT" + + @pytest.mark.asyncio + async def test_empty_query_does_not_raise_index_error(self): + """An empty 'query' string in the LLM response must not crash. + + Regression for issue #870: query.split()[0] raised IndexError + when the keyword-startswith guard was bypassed. Parsing now + returns None instead of crashing. + """ + prompt_service = MagicMock() + prompt_service.generate_sparql = AsyncMock(return_value={ + "query": "", + "explanation": "empty", + }) + generator = SPARQLGenerator(prompt_service=prompt_service) + + result = await generator._generate_with_llm( + _make_question_components(), _make_ontology_subset(), + ) + + assert result is None + + @pytest.mark.asyncio + async def test_whitespace_only_query_does_not_raise_index_error(self): + """A whitespace-only 'query' string must not crash. + + After .strip() the value becomes empty; the keyword-startswith + guard rejects it, but the parsing code stays safe even if the + guard is ever weakened. Verifies the empty-parts defence added + for issue #870. + """ + prompt_service = MagicMock() + prompt_service.generate_sparql = AsyncMock(return_value={ + "query": " \n\t ", + "explanation": "whitespace", + }) + generator = SPARQLGenerator(prompt_service=prompt_service) + + result = await generator._generate_with_llm( + _make_question_components(), _make_ontology_subset(), + ) + + assert result is None diff --git a/trustgraph-flow/trustgraph/query/ontology/sparql_generator.py b/trustgraph-flow/trustgraph/query/ontology/sparql_generator.py index 44c7e0a1..b143843e 100644 --- a/trustgraph-flow/trustgraph/query/ontology/sparql_generator.py +++ b/trustgraph-flow/trustgraph/query/ontology/sparql_generator.py @@ -203,10 +203,13 @@ ASK {{ if response and isinstance(response, dict): query = response.get('query', '').strip() if query.upper().startswith(('SELECT', 'ASK', 'CONSTRUCT', 'DESCRIBE')): + parts = query.split() + if not parts: + return None return SPARQLQuery( query=query, variables=self._extract_variables(query), - query_type=query.split()[0].upper(), + query_type=parts[0].upper(), explanation=response.get('explanation', 'Generated by LLM'), complexity_score=self._calculate_complexity(query) )