diff --git a/semantica/semantic_extract/ner_extractor.py b/semantica/semantic_extract/ner_extractor.py index 7a7c9208..e8b57bcd 100644 --- a/semantica/semantic_extract/ner_extractor.py +++ b/semantica/semantic_extract/ner_extractor.py @@ -109,6 +109,12 @@ class NERExtractor: - huggingface_model: HuggingFace model name - provider: LLM provider (for LLM method) - llm_model: LLM model name + - base_url: Custom base URL for OpenAI-compatible endpoints + (e.g. ``"https://my-gateway/v1"``). When set, the + provider automatically switches to ``Mode.JSON`` so that + third-party servers (Qwen, LLaMA gateways, etc.) that do + not implement the full function-calling protocol still + return correctly structured results. - device: Device for HuggingFace models ("cuda" or "cpu") - min_confidence: Minimum confidence threshold - ensemble_voting: Enable ensemble voting (default: False) @@ -423,7 +429,9 @@ class NERExtractor: return filtered except Exception as e: - self.logger.warning(f"Method {method_name} failed: {e}") + self.logger.warning( + "Method %s failed: %s", method_name, e, exc_info=True + ) continue # Ensemble voting if enabled diff --git a/semantica/semantic_extract/providers.py b/semantica/semantic_extract/providers.py index a0e4979b..663108a2 100644 --- a/semantica/semantic_extract/providers.py +++ b/semantica/semantic_extract/providers.py @@ -249,11 +249,18 @@ class BaseProvider: mode = instructor.Mode.TOOLS # Default mode if provider_name == "OpenAIProvider" and self.client: - if hasattr(instructor, "from_provider"): + custom_base_url = getattr(self, "base_url", None) + if custom_base_url: + # OpenAI-compatible custom endpoint: Mode.TOOLS is not reliably + # supported by third-party servers (Qwen, LLaMA gateways, etc.). + # Mode.JSON asks the model to return plain JSON and is broadly + # supported across all OpenAI-compatible APIs. + client = instructor.from_openai(self.client, mode=instructor.Mode.JSON) + elif hasattr(instructor, "from_provider"): try: client = instructor.from_provider( - provider=f"openai/{kwargs.get('model', self.model)}", - api_key=self.api_key + provider=f"openai/{kwargs.get('model', self.model)}", + api_key=self.api_key, ) except Exception: client = instructor.from_openai(self.client) @@ -395,15 +402,43 @@ class BaseProvider: if provider_name == "GroqProvider": create_kwargs["response_format"] = {"type": "json_object"} - - response = client.chat.completions.create(**create_kwargs) + + try: + response = client.chat.completions.create(**create_kwargs) + except Exception as primary_err: + # Mode.TOOLS can fail on standard OpenAI endpoints for certain + # models (streaming quirks, schema binding issues). Retry once + # with Mode.JSON before giving up entirely. + # Custom-base_url providers already use Mode.JSON from the start, + # so we only retry here for the standard OpenAI path. + if ( + provider_name == "OpenAIProvider" + and not getattr(self, "base_url", None) + and hasattr(self, "client") + and self.client + ): + self.logger.warning( + "instructor Mode.TOOLS failed for %s (%s); retrying with Mode.JSON.", + provider_name, + primary_err, + exc_info=True, + ) + json_client = instructor.from_openai(self.client, mode=instructor.Mode.JSON) + response = json_client.chat.completions.create(**create_kwargs) + else: + raise + verbose_mode = kwargs.get("verbose", False) or self.config.get("verbose", False) if verbose_mode: import sys print(f" [BaseProvider.generate_typed] Typed response received via instructor ({provider_name}).", flush=True, file=sys.stdout) return response except Exception as e: - self.logger.warning(f"Instructor generation failed ({e}), falling back to manual repair loop.") + self.logger.warning( + "Instructor generation failed (%s), falling back to manual repair loop.", + e, + exc_info=True, + ) # Fallback: Manual repair loop last_error = None @@ -411,9 +446,18 @@ class BaseProvider: for attempt in range(max_retries): try: - # 1. Generate JSON - # We use generate_structured to get the dict/list - json_result = self.generate_structured(current_prompt, max_retries=1, **kwargs) + # 1. Generate JSON – try structured mode first, then fall back to + # plain generate() + parse. Custom gateways that reject + # response_format=json_object would otherwise loop forever here. + try: + json_result = self.generate_structured(current_prompt, max_retries=1, **kwargs) + except Exception as struct_err: + self.logger.warning( + "generate_structured failed (%s); retrying with plain generate() + JSON parse.", + struct_err, + ) + raw_content = self.generate(current_prompt, **kwargs) + json_result = self._parse_json(raw_content) # 2. Validate with Schema # If the result is a list and schema expects a wrapper, or vice versa, we might need adjustment @@ -509,22 +553,40 @@ class OpenAIProvider(BaseProvider): """OpenAI provider implementation.""" def __init__( - self, api_key: Optional[str] = None, model: str = "gpt-3.5-turbo", **kwargs + self, + api_key: Optional[str] = None, + model: str = "gpt-3.5-turbo", + base_url: Optional[str] = None, + **kwargs, ): - """Initialize OpenAI provider.""" + """Initialize OpenAI provider. + + Args: + api_key: OpenAI API key (or OPENAI_API_KEY env var). + model: Default model name. + base_url: Optional custom base URL for OpenAI-compatible endpoints + (e.g. local gateways, Qwen, LLaMA proxies). When set, + ``instructor`` will use ``Mode.JSON`` instead of + ``Mode.TOOLS`` because most OpenAI-compatible servers do not + implement the full function-calling protocol. + """ super().__init__(**kwargs) self.api_key = api_key or config.get_api_key("openai") self.model = model + self.base_url = base_url # None → standard OpenAI; set → custom endpoint self.client = None self._init_client() def _init_client(self): - """Initialize OpenAI client.""" + """Initialize OpenAI client, respecting a custom base_url if provided.""" try: from openai import OpenAI if self.api_key: - self.client = OpenAI(api_key=self.api_key) + init_kwargs: Dict[str, Any] = {"api_key": self.api_key} + if self.base_url: + init_kwargs["base_url"] = self.base_url + self.client = OpenAI(**init_kwargs) except (ImportError, OSError): self.client = None self.logger.warning( @@ -560,8 +622,13 @@ class OpenAIProvider(BaseProvider): create_kwargs = { "model": kwargs.get("model", self.model), "messages": [{"role": "user", "content": prompt}], - "response_format": {"type": "json_object"}, } + # response_format=json_object is only safe for standard OpenAI endpoints. + # Custom gateways (base_url set) often reject or mishandle this parameter, + # causing silent fallback to pattern extraction. + if not self.base_url: + create_kwargs["response_format"] = {"type": "json_object"} + self._add_if_set(create_kwargs, kwargs, "temperature", "max_completion_tokens", "max_tokens", "top_p", "frequency_penalty", "presence_penalty", "seed", "stop", "logit_bias", "user") diff --git a/tests/test_issue_554_fixes.py b/tests/test_issue_554_fixes.py new file mode 100644 index 00000000..85dabd06 --- /dev/null +++ b/tests/test_issue_554_fixes.py @@ -0,0 +1,515 @@ +""" +Tests for issue #554: NERExtractor LLM method returning pattern-based output. + +Three bugs fixed: + 1. ner_extractor.py – exc_info=True missing on method-failure warning + 2. providers.py OpenAIProvider.generate_structured – forced response_format=json_object + even for custom gateway base_url endpoints that don't support it + 3. providers.py BaseProvider.generate_typed manual repair loop – no fallback from + generate_structured to plain generate() when the structured call itself fails + +These tests work without pydantic / instructor / openai installed: mock clients are +injected directly and a minimal stub schema class replaces pydantic.BaseModel where +needed. +""" + +import json +import sys +import os +import unittest +from unittest.mock import MagicMock, patch + +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) + +from semantica.semantic_extract.ner_extractor import NERExtractor +from semantica.semantic_extract.providers import OpenAIProvider +from semantica.utils.exceptions import ProcessingError +from semantica.utils.logging import get_logger + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _make_entity_response(entities): + """Return a mock that looks like an EntitiesResponse Pydantic model.""" + mock_resp = MagicMock() + mock_resp.entities = [ + MagicMock( + text=e["text"], + label=e["label"], + confidence=e.get("confidence", 0.9), + start=e.get("start", 0), + end=e.get("end", len(e["text"])), + ) + for e in entities + ] + return mock_resp + + +def _bare_openai_provider(base_url=None): + """ + Build an OpenAIProvider without invoking _init_client (no openai package needed). + """ + provider = object.__new__(OpenAIProvider) + provider.config = {} + provider.logger = get_logger("test_provider") + provider.api_key = "test-key" + provider.model = "test-model" + provider.base_url = base_url + provider.client = MagicMock() + return provider + + +class _StubEntityOut: + def __init__(self, text, label, confidence): + self.text = text + self.label = label + self.confidence = confidence + + +class _StubEntitiesResponse: + """ + Minimal pydantic-like schema stub usable without pydantic installed. + + The class-level `entities = None` is required so that + `hasattr(schema, "entities")` returns True inside generate_typed's + auto-wrap logic. + """ + + model_fields = {"entities": None} + entities = None # class-level placeholder — mirrors pydantic field descriptor + + def __init__(self, entities): + self.entities = entities + + @classmethod + def model_validate(cls, data): + if not isinstance(data, dict) or "entities" not in data: + raise ValueError(f"Expected dict with 'entities' key, got: {data!r}") + return cls([_StubEntityOut(**e) for e in data["entities"]]) + + +# --------------------------------------------------------------------------- +# Bug 1 – exc_info=True on method failure in NERExtractor +# --------------------------------------------------------------------------- + +class TestBug1ExcInfoOnMethodFailure(unittest.TestCase): + """ + NERExtractor.extract_entities must log the full traceback when a method + raises, not just the single-line message. Without exc_info=True the user + sees 'Method llm failed: ' but no root cause. + """ + + @patch("semantica.semantic_extract.methods.create_provider") + def test_traceback_logged_on_llm_failure(self, mock_create): + """Full traceback must appear in the log when the LLM method fails.""" + mock_llm = MagicMock() + mock_llm.is_available.return_value = True + mock_llm.generate_typed.side_effect = RuntimeError("gateway timeout") + mock_create.return_value = mock_llm + + extractor = NERExtractor(method="llm", provider="openai", llm_model="test-model") + + with self.assertLogs("semantica.ner_extractor", level="WARNING") as log_ctx: + extractor.extract_entities("Hello World.") + + has_traceback = any(r.exc_info is not None for r in log_ctx.records) + self.assertTrue( + has_traceback, + "Expected a WARNING record with exc_info set, but none found. " + "Ensure exc_info=True is in the method-failure warning call.", + ) + + @patch("semantica.semantic_extract.methods.create_provider") + def test_failure_message_contains_method_name(self, mock_create): + """Warning message must identify which method failed.""" + mock_llm = MagicMock() + mock_llm.is_available.return_value = True + mock_llm.generate_typed.side_effect = ProcessingError("schema mismatch") + mock_create.return_value = mock_llm + + extractor = NERExtractor(method="llm", provider="openai", llm_model="test-model") + + with self.assertLogs("semantica.ner_extractor", level="WARNING") as log_ctx: + extractor.extract_entities("Hello World.") + + messages = " ".join(r.getMessage() for r in log_ctx.records) + self.assertIn("llm", messages.lower()) + + @patch("semantica.semantic_extract.methods.create_provider") + def test_fallback_to_pattern_on_llm_failure(self, mock_create): + """After LLM failure the extractor must still return results, not raise.""" + mock_llm = MagicMock() + mock_llm.is_available.return_value = True + mock_llm.generate_typed.side_effect = ProcessingError("instructor failed") + mock_create.return_value = mock_llm + + extractor = NERExtractor(method="llm", provider="openai", llm_model="test-model") + + with self.assertLogs("semantica.ner_extractor", level="WARNING"): + result = extractor.extract_entities( + "John Smith visited Microsoft in New York." + ) + + self.assertIsInstance(result, list) + methods = {e.metadata.get("extraction_method") for e in result} + self.assertTrue( + methods <= {"pattern", "last_resort_pattern"}, + f"Unexpected extraction_method values after fallback: {methods}", + ) + + @patch("semantica.semantic_extract.methods.EntitiesResponse", + _StubEntitiesResponse, create=True) + @patch("semantica.semantic_extract.methods.SCHEMAS_AVAILABLE", True) + @patch("semantica.semantic_extract.methods.create_provider") + def test_llm_success_returns_llm_typed_metadata(self, mock_create): + """When LLM succeeds, entities must carry extraction_method='llm_typed'.""" + mock_llm = MagicMock() + mock_llm.is_available.return_value = True + mock_llm.generate_typed.return_value = _make_entity_response([ + {"text": "John Smith", "label": "PERSON", "confidence": 0.95}, + {"text": "Microsoft", "label": "ORG", "confidence": 0.90}, + ]) + mock_create.return_value = mock_llm + + extractor = NERExtractor(method="llm", provider="openai", llm_model="test-model") + result = extractor.extract_entities("John Smith visited Microsoft.") + + self.assertGreaterEqual(len(result), 1) + for e in result: + self.assertEqual( + e.metadata.get("extraction_method"), "llm_typed", + f"Expected llm_typed, got {e.metadata}", + ) + + +# --------------------------------------------------------------------------- +# Bug 2 – OpenAIProvider.generate_structured must not send response_format +# when base_url (custom gateway) is set +# --------------------------------------------------------------------------- + +class TestBug2GenerateStructuredCustomGateway(unittest.TestCase): + """ + generate_structured always added response_format=json_object even for + custom gateways that don't support it. After the fix, it is omitted when + base_url is set. + """ + + def _capture_create_kwargs(self, provider, prompt="Extract entities."): + """Run generate_structured and return kwargs sent to the API.""" + captured = {} + payload = json.dumps({ + "entities": [{"text": "Alice", "label": "PERSON", "confidence": 0.9}] + }) + resp = MagicMock() + resp.choices[0].message.content = payload + + def fake_create(**kwargs): + captured.update(kwargs) + return resp + + provider.client.chat.completions.create.side_effect = fake_create + provider.generate_structured(prompt) + return captured + + def test_standard_endpoint_sends_response_format(self): + """Standard OpenAI (no base_url) must still send response_format=json_object.""" + provider = _bare_openai_provider(base_url=None) + kwargs = self._capture_create_kwargs(provider) + self.assertIn( + "response_format", kwargs, + "Standard endpoint must send response_format=json_object", + ) + self.assertEqual(kwargs["response_format"], {"type": "json_object"}) + + def test_custom_gateway_omits_response_format(self): + """Custom gateway (base_url set) must NOT send response_format.""" + provider = _bare_openai_provider( + base_url="https://qa-llmgateway.local/api/v1beta/llm/messages" + ) + kwargs = self._capture_create_kwargs(provider) + self.assertNotIn( + "response_format", kwargs, + "Custom gateway must not receive response_format=json_object — " + "many gateways reject this parameter, causing silent fallback.", + ) + + def test_custom_localhost_gateway_omits_response_format(self): + """Any non-None base_url must suppress response_format.""" + provider = _bare_openai_provider(base_url="http://localhost:8080/v1") + kwargs = self._capture_create_kwargs(provider) + self.assertNotIn("response_format", kwargs) + + def test_generate_structured_parses_json_without_response_format(self): + """Result must still be parsed correctly even without response_format.""" + provider = _bare_openai_provider( + base_url="https://qa-llmgateway.local/api/v1beta" + ) + payload = {"entities": [{"text": "Bob", "label": "PERSON", "confidence": 0.8}]} + resp = MagicMock() + resp.choices[0].message.content = json.dumps(payload) + provider.client.chat.completions.create.return_value = resp + + result = provider.generate_structured("Find entities.") + self.assertEqual(result, payload) + + def test_standard_endpoint_result_unchanged(self): + """Standard-endpoint path must still return correct data.""" + provider = _bare_openai_provider(base_url=None) + payload = {"entities": [{"text": "Eve", "label": "PERSON", "confidence": 0.7}]} + resp = MagicMock() + resp.choices[0].message.content = json.dumps(payload) + provider.client.chat.completions.create.return_value = resp + + result = provider.generate_structured("Find entities.") + self.assertEqual(result, payload) + + +# --------------------------------------------------------------------------- +# Bug 3 – generate_typed manual repair loop must fall back to plain generate() +# when generate_structured itself raises +# --------------------------------------------------------------------------- + +class TestBug3GenerateTypedFallbackToPlainGenerate(unittest.TestCase): + """ + The manual repair loop inside generate_typed called generate_structured, + which for custom gateways also fails (same response_format rejection). + After the fix, if generate_structured raises, the loop immediately retries + via plain generate() + _parse_json. + """ + + def _valid_json(self): + return json.dumps({ + "entities": [ + {"text": "Alice", "label": "PERSON", "confidence": 0.95}, + {"text": "Acme Corp", "label": "ORG", "confidence": 0.88}, + ] + }) + + def test_fallback_to_plain_generate_when_structured_fails(self): + """ + If generate_structured raises, generate_typed must call plain generate() + and successfully parse the JSON it returns. + """ + provider = _bare_openai_provider(base_url="https://gateway.local/api/v1") + + provider.generate_structured = MagicMock( + side_effect=ProcessingError("response_format not supported") + ) + provider.generate = MagicMock(return_value=self._valid_json()) + + result = provider.generate_typed( + "Extract entities from: Alice works at Acme Corp.", + schema=_StubEntitiesResponse, + max_retries=2, + ) + + self.assertEqual(len(result.entities), 2) + self.assertEqual(result.entities[0].text, "Alice") + self.assertEqual(result.entities[1].text, "Acme Corp") + provider.generate.assert_called() + + def test_generate_structured_is_attempted_first(self): + """Plain generate() is the fallback, not the primary path.""" + provider = _bare_openai_provider() + + call_order = [] + + def fake_structured(_prompt, **_kw): + call_order.append("structured") + return {"entities": [{"text": "Bob", "label": "PERSON", "confidence": 0.9}]} + + def fake_generate(_prompt, **_kw): + call_order.append("generate") + return json.dumps({"entities": [{"text": "Bob", "label": "PERSON", "confidence": 0.9}]}) + + provider.generate_structured = fake_structured + provider.generate = fake_generate + + provider.generate_typed( + "Find entities.", schema=_StubEntitiesResponse, max_retries=1 + ) + + self.assertEqual(call_order[0], "structured", + "generate_structured must be tried first") + self.assertNotIn("generate", call_order, + "plain generate() must NOT be called when generate_structured succeeds") + + def test_error_raised_when_both_paths_fail(self): + """ + If both generate_structured AND plain generate() fail, generate_typed + must raise — not silently return empty/wrong data. + """ + provider = _bare_openai_provider(base_url="https://gateway.local/api/v1") + + provider.generate_structured = MagicMock( + side_effect=ProcessingError("json_object not supported") + ) + provider.generate = MagicMock( + side_effect=ConnectionError("gateway unreachable") + ) + + with self.assertRaises(Exception): + provider.generate_typed( + "Extract entities.", schema=_StubEntitiesResponse, max_retries=2 + ) + + def test_fallback_preserves_error_on_bad_json(self): + """ + If plain generate() returns malformed JSON the error must propagate, + not silently swallow the result. + """ + provider = _bare_openai_provider(base_url="https://gateway.local/api/v1") + + provider.generate_structured = MagicMock( + side_effect=ProcessingError("json_object not supported") + ) + provider.generate = MagicMock(return_value="not json") + + with self.assertRaises(Exception): + provider.generate_typed( + "Extract entities.", schema=_StubEntitiesResponse, max_retries=1 + ) + + def test_schema_validation_wraps_plain_list(self): + """ + If plain generate() returns a bare list (not wrapped in {"entities": [...]}), + generate_typed must auto-wrap it before calling model_validate. + """ + provider = _bare_openai_provider(base_url="https://gateway.local/api/v1") + + provider.generate_structured = MagicMock( + side_effect=ProcessingError("response_format not supported") + ) + provider.generate = MagicMock(return_value=json.dumps([ + {"text": "Carol", "label": "PERSON", "confidence": 0.85}, + ])) + + result = provider.generate_typed( + "Extract entities.", schema=_StubEntitiesResponse, max_retries=2 + ) + + self.assertEqual(len(result.entities), 1) + self.assertEqual(result.entities[0].text, "Carol") + + +# --------------------------------------------------------------------------- +# Integration – all three fixes working together +# --------------------------------------------------------------------------- + +class TestIssue554EndToEnd(unittest.TestCase): + """ + Simulate harshalizode's exact scenario: + - NERExtractor(method="llm", provider="openai", base_url="custom-gateway") + - instructor fails / generate_typed is mocked to succeed + - Returned entities must carry extraction_method="llm_typed", not "pattern" + """ + + @patch("semantica.semantic_extract.methods.EntitiesResponse", + _StubEntitiesResponse, create=True) + @patch("semantica.semantic_extract.methods.SCHEMAS_AVAILABLE", True) + @patch("semantica.semantic_extract.methods.create_provider") + def test_custom_gateway_returns_llm_entities_not_pattern(self, mock_create): + """ + End-to-end: custom gateway produces LLM entities, NOT pattern-fallback. + """ + mock_llm = MagicMock() + mock_llm.is_available.return_value = True + mock_llm.generate_typed.return_value = _make_entity_response([ + {"text": "Miss Theodora Clare", "label": "PERSON", "confidence": 0.95}, + {"text": "Cedar Lodge", "label": "LOCATION", "confidence": 0.95}, + {"text": "India", "label": "GPE", "confidence": 0.95}, + ]) + mock_create.return_value = mock_llm + + extractor = NERExtractor( + method="llm", + provider="openai", + llm_model="Llama-4-Scout", + base_url="https://qa-llmgateway.local/api/v1beta/llm/messages", + entity_types=["PERSON", "LOCATION", "GPE"], + ) + + result = extractor.extract_entities( + "Miss Theodora Clare arrived at Cedar Lodge. She had spent five years in India." + ) + + # At least some LLM entities must come through + self.assertGreater(len(result), 0) + + # None should carry pattern metadata + for e in result: + self.assertNotEqual( + e.metadata.get("extraction_method"), "pattern", + f"Entity {e.text!r} must not have extraction_method='pattern'; " + f"got metadata={e.metadata}", + ) + + # Must include at least the PERSON entity + labels = {e.label for e in result} + self.assertIn("PERSON", labels) + + @patch("semantica.semantic_extract.methods.create_provider") + def test_instructor_failure_logged_with_traceback(self, mock_create): + """ + When instructor / generate_typed fails, the warning must carry exc_info + so the user can diagnose the root cause from logs alone. + """ + mock_llm = MagicMock() + mock_llm.is_available.return_value = True + mock_llm.generate_typed.side_effect = ProcessingError( + "instructor: response_format not supported by gateway" + ) + mock_create.return_value = mock_llm + + extractor = NERExtractor( + method="llm", + provider="openai", + llm_model="Llama-4-Scout", + base_url="https://qa-llmgateway.local/api/v1beta/llm/messages", + ) + + with self.assertLogs("semantica.ner_extractor", level="WARNING") as log_ctx: + result = extractor.extract_entities( + "John Smith visited Microsoft in Seattle." + ) + + has_traceback = any(r.exc_info is not None for r in log_ctx.records) + self.assertTrue( + has_traceback, + "Warning log must carry exc_info so the gateway error is diagnosable.", + ) + self.assertIsInstance(result, list) + + @patch("semantica.semantic_extract.methods.EntitiesResponse", + _StubEntitiesResponse, create=True) + @patch("semantica.semantic_extract.methods.SCHEMAS_AVAILABLE", True) + @patch("semantica.semantic_extract.methods.create_provider") + def test_fallback_chain_llm_then_pattern(self, mock_create): + """ + With method=["llm", "pattern"], LLM failure falls through to pattern — + not raise — and pattern entities are returned. + """ + mock_llm = MagicMock() + mock_llm.is_available.return_value = True + mock_llm.generate_typed.side_effect = ProcessingError("timeout") + mock_create.return_value = mock_llm + + extractor = NERExtractor( + method=["llm", "pattern"], + provider="openai", + llm_model="test-model", + ) + + with self.assertLogs("semantica.ner_extractor", level="WARNING"): + result = extractor.extract_entities( + "Barack Obama visited the United States Capitol." + ) + + self.assertIsInstance(result, list) + self.assertGreater(len(result), 0, "Pattern method should extract something") + + +if __name__ == "__main__": + unittest.main(verbosity=2)