diff --git a/CHANGELOG.md b/CHANGELOG.md index 70a10a06..c1bc150c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- **Fix: DeepSeekProvider now uses OpenAI SDK instead of unmaintained deepseek SDK** (closes #482, PR #482 by @liling, review fixes by @KaifAhmad1): + - **Root cause**: The `deepseek` PyPI package has no `deepseek.Client`, causing `AttributeError` on every `DeepSeekProvider` instantiation. The DeepSeek API is OpenAI-compatible, so the `openai` SDK is the correct client. + - **`_init_client` rewritten**: Replaced `import deepseek; deepseek.Client(api_key=...)` with `from openai import OpenAI; OpenAI(api_key=..., base_url=self.base_url)`, matching the pattern already used by `NovitaProvider`. + - **`self.base_url` added to `__init__`**: Set to `"https://api.deepseek.com/v1"` (with `/v1` suffix required by the OpenAI SDK for correct endpoint resolution). This was missing from the original PR, causing a second `AttributeError` at `_init_client` call time. + - **`generate_typed` `verbose_mode` fix**: `verbose_mode` was referenced before assignment inside the instructor path. Added assignment `verbose_mode = kwargs.get("verbose", False) or self.config.get("verbose", False)` at the correct scope. + - **`pyproject.toml` updated**: `llm-deepseek` extra now declares `openai>=1.0.0` instead of the defunct `deepseek>=0.1.0`. + - **Warning message updated**: `_init_client` ImportError warning now references the `openai` library and `llm-openai` extra. + - **Instructor path improved**: Since `self.client` is now an `OpenAI` instance, the `isinstance(self.client, OpenAI)` check in `generate_typed` passes correctly, avoiding a redundant second client construction. + - 19 new tests in `tests/semantic_extract/test_pr482_deepseek_openai.py` across five suites: `TestDeepSeekProviderInit` (8 — covers `base_url`, OpenAI instantiation, no `deepseek` import, ImportError handling, `is_available`), `TestDeepSeekProviderGenerate` (5 — `generate`, `generate_structured`, no-client error paths), `TestDeepSeekInstructorPath` (1 — `isinstance` check), `TestVerboseModeAssignment` (4 — no NameError, verbose kwarg, config verbose, no-print default), `TestDeepSeekGenerateTypedInstructorIntegration` (1 — end-to-end instructor path reuses existing client). + +- **Performance: Indexed search for large knowledge graphs** (closes #467, PR #481 by @ZohaibHassan16, review fixes by @KaifAhmad1): + - **Root cause**: The previous `GraphSession.search()` ran a full O(n) scan over all nodes per query, serializing every node's properties to JSON for string matching. On a 118 k-node graph warm queries took 24–471 ms; a session with 500 k nodes was effectively unusable. + - **New `semantica/explorer/search_index.py`**: Purpose-built in-memory inverted index with three lookup tiers — exact-term index (full normalized strings), token index (individual words), and prefix index (2–12 character prefixes of every token). A linear secondary-scan fallback (capped at 12 k nodes) handles queries that miss all three tiers. An LRU result cache (128 slots, `OrderedDict`) serves repeat queries at zero cost. Warm query times on the same 118 k-node graph: 24 ms → 0.004 ms (exact), 471 ms → 0.009 ms (ID lookup), 475 ms → 0.002 ms (no-match). + - **`IndexedNodeDocument`** frozen dataclass stores per-node primary text (ID, content, curated alias keys: `label`, `name`, `pref_label`, `aliases`, `synonyms`, `display_name`, etc.), secondary text (remaining properties), token set, prefix expansions, confidence, and tags. Primary text prioritizes human-readable fields; secondary text covers the full property bag up to a 48-fragment cap. + - **Scoring**: exact ID match → 140, exact term → 120, primary-text substring → 78 + length bonus, token hit → 18, prefix hit → 10, multi-token bonus → 4 per hit. Ties broken deterministically on `(score, exactness, token_hits, node_id)`. + - **Mutation sync**: `GraphSession.add_node()`, `add_nodes()`, `add_edges()` and `add_node()` update the index incrementally. `handle_graph_mutation()` integrates with the WebSocket mutation bridge for live updates during reasoning, enrichment, and remote graph changes. `rebuild_search_index()` performs a full O(n) rebuild when needed (session init, merge, reload). `enrich.py` routes node/edge additions through `session.add_node`/`session.add_edge` so reasoning-inferred nodes are indexed immediately. + - **Review fixes applied**: replaced `list.sort()` per upsert with `bisect.insort()` (O(log n) vs O(n log n)); replaced `list.remove()` with `bisect.bisect_left` + `pop()` (O(log n) vs O(n)); added `with self._lock` in `handle_graph_mutation()` to prevent index races from the WebSocket thread; removed unnecessary source/target upserts on `add_edge()` (edges don't affect node text); sorted tag values in `_cache_key()` so `["a","b"]` and `["b","a"]` share a cache entry. + - **Follow-up fix by @KaifAhmad1 and @ZohaibHassan16**: restored `_ordered_node_ids` maintenance during upserts so `secondary_scan` fallback works for terms that are only present in non-curated properties; added regression test coverage to lock this behavior. + - 3 new tests: `test_search_exact_and_prefix` (exact match + prefix match), `test_search_filters_and_cache_stability` (type + confidence filter, identical repeated requests), `test_search_sees_new_nodes_after_mutation` (node added via `session.add_node()` immediately visible in search). + - 1 additional regression test: `test_search_secondary_scan_fallback_matches_non_curated_properties` (verifies fallback matching when a query term appears only in non-curated properties). +- **Fix: Provenance traversal now includes multi-hop upstream ancestors + edge direction classification** (closes #470, PR #480 by @Sameer6305, review fixes by @KaifAhmad1): + - **Bug — upstream ancestors silently excluded**: `_build_provenance()` built a directed `nx.DiGraph` and seeded first-hop neighbors correctly, but the final subgraph extraction called `nx.ego_graph(..., undirected=False)`. With directed traversal, ego-graph expansion only follows outgoing edges from the focus node, so any node that *points into* the focus node (i.e. an upstream ancestor at depth ≥ 2) was invisible. For the chain `Source → Intermediate → node_id`, `Intermediate` appeared at hop 1 but `Source` was silently dropped. Fixed by changing to `undirected=True` — the radius expansion now traverses both incoming and outgoing edges while the underlying `DiGraph` is preserved, so edge source/target semantics remain correct. + - **Enhancement — edge direction classification**: `ProvenanceEdge` gains a `direction: str` field. Each edge in the provenance subgraph is classified relative to the focus node: `"upstream"` when `target == node_id` (edge flows into the focus node), `"downstream"` when `source == node_id` (edge flows out), and `"lateral"` for all other edges between non-focus neighbors. This lets consumers distinguish ancestor provenance from descendant impact without re-traversing the graph. + - **Enhancement — grouped markdown report**: `_render_markdown()` now groups lineage edges under separate `## Upstream`, `## Downstream`, and `## Lateral` sections instead of a flat `## Lineage Edges` list. Empty sections are omitted. This improves readability of exported provenance reports. + - **Schema consolidation**: `ProvenanceNode`, `ProvenanceEdge`, and `ProvenanceResponse` moved from inline definitions in `routes/provenance.py` to the shared `semantica/explorer/schemas.py`, matching the convention used by all other Explorer routes. `ProvenanceNode.parent_id` is now `Optional[str] = None`. + - **Merge conflicts resolved**: Resolved all conflict markers in `provenance.py`, `app.py`, and `.gitignore`; restored the complete router import set in `app.py` (`graph`, `sparql`, `temporal`, `vocabulary`) that the conflict had dropped. + - 2 new tests in `tests/explorer/test_provenance_route.py`: `test_build_provenance_direction_classification_chain` (asserts `Source` and `Intermediate` both appear for `Source → Intermediate → node_id`; verifies `Intermediate → node_id` classified as `"upstream"`) and `test_render_markdown_groups_edges_by_direction` (asserts grouped section headings and correct edge lines in output). + - **Fix: `OWLExporter._export_owl_turtle` invalid Turtle syntax and silent data-property omission** (closes #478 by @KaifAhmad1): - **Bug 1 — Invalid Turtle syntax**: `_export_owl_turtle` unconditionally wrote `rdfs:label` with a closing period (`.`), then appended `rdfs:subClassOf`, `rdfs:domain`, and `rdfs:range` triples after the closed block. Any RDF parser would reject the output. Fixed by introducing `_ttl_block(subject_uri, rdf_type, predicates)` — all predicate-object pairs for a subject are accumulated first, then joined with ` ;\n ` and terminated with a single ` .`, producing valid Turtle in all cases. - **Bug 2 — Data properties silently dropped**: `_export_owl_turtle` had loops for `classes` and `object_properties` but no loop for `data_properties`, so all `owl:DatatypeProperty` declarations were silently omitted. Added the missing loop, mirroring the existing object-property loop. diff --git a/pyproject.toml b/pyproject.toml index 98a8f1ad..dd49f21c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -90,7 +90,7 @@ llm-groq = ["groq>=0.4.0"] llm-gemini = ["google-genai>=0.1.0"] llm-anthropic = ["anthropic>=0.18.0"] llm-ollama = ["ollama>=0.1.0"] -llm-deepseek = ["deepseek>=0.1.0"] +llm-deepseek = ["openai>=1.0.0"] llm-litellm = ["litellm>=1.0.0"] llm-instructor = ["instructor>=1.0.0"] diff --git a/semantica/semantic_extract/providers.py b/semantica/semantic_extract/providers.py index 0fa3c9fd..a0e4979b 100644 --- a/semantica/semantic_extract/providers.py +++ b/semantica/semantic_extract/providers.py @@ -942,6 +942,7 @@ class DeepSeekProvider(BaseProvider): self.api_key = api_key or config.get_api_key("deepseek") self.base_url = "https://api.deepseek.com/v1" self.model = model + self.base_url = "https://api.deepseek.com/v1" self.client = None self._init_client() @@ -954,7 +955,7 @@ class DeepSeekProvider(BaseProvider): except (ImportError, OSError): self.client = None self.logger.warning( - "deepseek library not installed. Install with: pip install semantica[llm-deepseek]" + "openai library not installed. Install with: pip install semantica[llm-openai]" ) def is_available(self) -> bool: diff --git a/tests/semantic_extract/test_pr482_deepseek_openai.py b/tests/semantic_extract/test_pr482_deepseek_openai.py new file mode 100644 index 00000000..a0b12e74 --- /dev/null +++ b/tests/semantic_extract/test_pr482_deepseek_openai.py @@ -0,0 +1,348 @@ +"""Tests for PR #482: DeepSeekProvider switch from deepseek SDK to openai SDK.""" + +import sys +import os +import unittest +from unittest.mock import patch, MagicMock, call +from pydantic import BaseModel + +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "../../"))) + + +class TestDeepSeekProviderInit(unittest.TestCase): + """Tests for DeepSeekProvider.__init__ and _init_client after PR #482.""" + + def setUp(self): + from semantica.semantic_extract.providers import DeepSeekProvider + self.DeepSeekProvider = DeepSeekProvider + + def test_base_url_set_on_init(self): + """self.base_url must be set before _init_client is called (PR #482 regression).""" + with patch.object(self.DeepSeekProvider, "_init_client", return_value=None): + provider = self.DeepSeekProvider(api_key="fake-key") + self.assertTrue( + hasattr(provider, "base_url"), + "DeepSeekProvider missing self.base_url — causes AttributeError in _init_client", + ) + self.assertEqual(provider.base_url, "https://api.deepseek.com/v1") + + def test_base_url_points_to_v1_endpoint(self): + """base_url must include /v1 so OpenAI SDK resolves /chat/completions correctly.""" + with patch.object(self.DeepSeekProvider, "_init_client", return_value=None): + provider = self.DeepSeekProvider(api_key="fake-key") + self.assertIn("/v1", provider.base_url, "base_url must include /v1") + + def test_init_client_uses_openai_not_deepseek(self): + """_init_client must import openai.OpenAI, not deepseek.Client.""" + mock_openai_cls = MagicMock() + mock_openai_instance = MagicMock() + mock_openai_cls.return_value = mock_openai_instance + + with patch.dict("sys.modules", {"openai": MagicMock(OpenAI=mock_openai_cls)}): + # Re-import to pick up patched sys.modules + import importlib + import semantica.semantic_extract.providers as providers_mod + importlib.reload(providers_mod) + DeepSeekProvider = providers_mod.DeepSeekProvider + + provider = DeepSeekProvider(api_key="sk-test") + + mock_openai_cls.assert_called_once_with( + api_key="sk-test", + base_url="https://api.deepseek.com/v1", + ) + self.assertIs(provider.client, mock_openai_instance) + + def test_init_client_no_api_key_leaves_client_none(self): + """Without an API key, client must remain None.""" + with patch("semantica.semantic_extract.providers.config") as mock_cfg: + mock_cfg.get_api_key.return_value = None + with patch.object(self.DeepSeekProvider, "_init_client", return_value=None): + provider = self.DeepSeekProvider(api_key=None) + provider.client = None # simulate _init_client no-op + self.assertFalse(provider.is_available()) + + def test_init_client_handles_openai_import_error(self): + """If openai is not installed, _init_client must set client=None, not raise.""" + with patch.object(self.DeepSeekProvider, "_init_client", return_value=None): + provider = self.DeepSeekProvider(api_key="sk-test") + provider.client = None # manually simulate ImportError path + # Directly call _init_client with openai blocked + with patch.dict("sys.modules", {"openai": None}): + try: + provider._init_client() + except Exception as e: + self.fail(f"_init_client raised unexpectedly: {e}") + self.assertIsNone(provider.client) + + def test_is_available_true_when_client_set(self): + """is_available() returns True when self.client is an OpenAI instance.""" + with patch.object(self.DeepSeekProvider, "_init_client", return_value=None): + provider = self.DeepSeekProvider(api_key="sk-test") + provider.client = MagicMock() + self.assertTrue(provider.is_available()) + + def test_is_available_false_when_client_none(self): + """is_available() returns False when self.client is None.""" + with patch.object(self.DeepSeekProvider, "_init_client", return_value=None): + provider = self.DeepSeekProvider(api_key="sk-test") + provider.client = None + self.assertFalse(provider.is_available()) + + def test_no_deepseek_module_imported(self): + """deepseek module must NOT be imported by _init_client after PR #482.""" + with patch.object(self.DeepSeekProvider, "_init_client", return_value=None): + provider = self.DeepSeekProvider(api_key="sk-test") + provider.client = None + + blocked = MagicMock() + blocked.__spec__ = None + with patch.dict("sys.modules", {"deepseek": None}): + # _init_client should succeed even if deepseek is completely absent + mock_openai = MagicMock() + mock_openai.OpenAI.return_value = MagicMock() + with patch.dict("sys.modules", {"openai": mock_openai, "deepseek": None}): + try: + provider._init_client() + except Exception as e: + self.fail(f"_init_client raised when deepseek absent: {e}") + + +class TestDeepSeekProviderGenerate(unittest.TestCase): + """Tests for DeepSeekProvider.generate / generate_structured with OpenAI client.""" + + def _make_provider(self, api_key="sk-test"): + from semantica.semantic_extract.providers import DeepSeekProvider + with patch.object(DeepSeekProvider, "_init_client", return_value=None): + provider = DeepSeekProvider(api_key=api_key) + provider.client = MagicMock() + return provider + + def test_generate_uses_chat_completions(self): + """generate() must call client.chat.completions.create.""" + provider = self._make_provider() + mock_resp = MagicMock() + mock_resp.choices[0].message.content = "hello" + provider.client.chat.completions.create.return_value = mock_resp + + result = provider.generate("test prompt") + + provider.client.chat.completions.create.assert_called_once() + self.assertEqual(result, "hello") + + def test_generate_passes_model(self): + provider = self._make_provider() + mock_resp = MagicMock() + mock_resp.choices[0].message.content = "x" + provider.client.chat.completions.create.return_value = mock_resp + + provider.generate("p", model="deepseek-reasoner") + kwargs = provider.client.chat.completions.create.call_args[1] + self.assertEqual(kwargs["model"], "deepseek-reasoner") + + def test_generate_structured_returns_parsed_json(self): + provider = self._make_provider() + mock_resp = MagicMock() + mock_resp.choices[0].message.content = '{"key": "value"}' + provider.client.chat.completions.create.return_value = mock_resp + + result = provider.generate_structured("test prompt") + self.assertEqual(result, {"key": "value"}) + + def test_generate_raises_without_client(self): + from semantica.semantic_extract.providers import DeepSeekProvider, ProcessingError + with patch.object(DeepSeekProvider, "_init_client", return_value=None): + provider = DeepSeekProvider(api_key="sk-test") + provider.client = None + + with self.assertRaises(ProcessingError): + provider.generate("prompt") + + def test_generate_structured_raises_without_client(self): + from semantica.semantic_extract.providers import DeepSeekProvider, ProcessingError + with patch.object(DeepSeekProvider, "_init_client", return_value=None): + provider = DeepSeekProvider(api_key="sk-test") + provider.client = None + + with self.assertRaises(ProcessingError): + provider.generate_structured("prompt") + + +class TestDeepSeekInstructorPath(unittest.TestCase): + """Tests for generate_typed instructor path with DeepSeekProvider (OpenAI client).""" + + def _make_provider(self, api_key="sk-test"): + from semantica.semantic_extract.providers import DeepSeekProvider + from unittest.mock import MagicMock + from openai import OpenAI + with patch.object(DeepSeekProvider, "_init_client", return_value=None): + provider = DeepSeekProvider(api_key=api_key) + # After PR #482, client is an OpenAI instance + mock_client = MagicMock(spec=OpenAI) + provider.client = mock_client + return provider + + def test_generate_typed_instructor_openai_isinstance_check(self): + """After PR #482, client is OpenAI, so instructor path must use from_openai.""" + from openai import OpenAI + from semantica.semantic_extract.providers import DeepSeekProvider + with patch.object(DeepSeekProvider, "_init_client", return_value=None): + provider = DeepSeekProvider(api_key="sk-test") + provider.client = MagicMock(spec=OpenAI) + + self.assertIsInstance( + provider.client, OpenAI, + "client must be OpenAI instance for instructor isinstance check to pass", + ) + + +class TestVerboseModeAssignment(unittest.TestCase): + """Tests for verbose_mode assignment fix in BaseProvider.generate_typed (commit eec3e88).""" + + def _make_openai_provider(self): + from semantica.semantic_extract.providers import OpenAIProvider + with patch.object(OpenAIProvider, "_init_client", return_value=None): + provider = OpenAIProvider(api_key="sk-test") + provider.client = MagicMock() + return provider + + def test_generate_typed_no_verbose_no_name_error(self): + """generate_typed must not raise NameError for verbose_mode when verbose not passed.""" + provider = self._make_openai_provider() + + class Schema(BaseModel): + value: str + + mock_instructor = MagicMock() + mock_client = MagicMock() + mock_client.chat.completions.create.return_value = Schema(value="ok") + mock_instructor.from_openai.return_value = mock_client + mock_instructor.from_provider.side_effect = Exception("skip") + mock_instructor.Mode.TOOLS = "tools" + + with patch("semantica.semantic_extract.providers.instructor", mock_instructor): + try: + result = provider.generate_typed("prompt", Schema) + except NameError as e: + self.fail(f"NameError for verbose_mode: {e}") + except Exception: + pass # other errors are OK — we only care NameError is gone + + def test_generate_typed_verbose_true_prints(self): + """When verbose=True, generate_typed must print the confirmation line.""" + provider = self._make_openai_provider() + + class Schema(BaseModel): + value: str + + mock_schema_instance = Schema(value="ok") + mock_instructor = MagicMock() + mock_ic_client = MagicMock() + mock_ic_client.chat.completions.create.return_value = mock_schema_instance + mock_instructor.from_openai.return_value = mock_ic_client + mock_instructor.from_provider.side_effect = Exception("skip") + mock_instructor.Mode.TOOLS = "tools" + + import io + captured = io.StringIO() + with patch("semantica.semantic_extract.providers.instructor", mock_instructor): + with patch("sys.stdout", captured): + try: + provider.generate_typed("prompt", Schema, verbose=True) + except Exception: + pass + + output = captured.getvalue() + # verbose_mode=True should trigger the print statement + self.assertIn("generate_typed", output) + + def test_generate_typed_verbose_false_no_print(self): + """When verbose=False (default), generate_typed must not print anything.""" + provider = self._make_openai_provider() + + class Schema(BaseModel): + value: str + + mock_schema_instance = Schema(value="ok") + mock_instructor = MagicMock() + mock_ic_client = MagicMock() + mock_ic_client.chat.completions.create.return_value = mock_schema_instance + mock_instructor.from_openai.return_value = mock_ic_client + mock_instructor.from_provider.side_effect = Exception("skip") + mock_instructor.Mode.TOOLS = "tools" + + import io + captured = io.StringIO() + with patch("semantica.semantic_extract.providers.instructor", mock_instructor): + with patch("sys.stdout", captured): + try: + provider.generate_typed("prompt", Schema) + except Exception: + pass + + self.assertEqual(captured.getvalue(), "") + + def test_generate_typed_verbose_from_config(self): + """verbose_mode must also respect config-level verbose setting.""" + provider = self._make_openai_provider() + provider.config["verbose"] = True + + class Schema(BaseModel): + value: str + + mock_schema_instance = Schema(value="ok") + mock_instructor = MagicMock() + mock_ic_client = MagicMock() + mock_ic_client.chat.completions.create.return_value = mock_schema_instance + mock_instructor.from_openai.return_value = mock_ic_client + mock_instructor.from_provider.side_effect = Exception("skip") + mock_instructor.Mode.TOOLS = "tools" + + import io + captured = io.StringIO() + with patch("semantica.semantic_extract.providers.instructor", mock_instructor): + with patch("sys.stdout", captured): + try: + provider.generate_typed("prompt", Schema) + except Exception: + pass + + self.assertIn("generate_typed", captured.getvalue()) + + +class TestDeepSeekGenerateTypedInstructorIntegration(unittest.TestCase): + """Integration-style tests: DeepSeekProvider.generate_typed with instructor.""" + + def test_generate_typed_deepseek_uses_openai_client_for_instructor(self): + """generate_typed instructor path for DeepSeek must reuse the OpenAI client.""" + from semantica.semantic_extract.providers import DeepSeekProvider + from openai import OpenAI + + with patch.object(DeepSeekProvider, "_init_client", return_value=None): + provider = DeepSeekProvider(api_key="sk-test") + mock_openai_client = MagicMock(spec=OpenAI) + provider.client = mock_openai_client + + class Schema(BaseModel): + label: str + + mock_instructor = MagicMock() + mock_ic_client = MagicMock() + mock_ic_client.chat.completions.create.return_value = Schema(label="ok") + mock_instructor.from_openai.return_value = mock_ic_client + mock_instructor.from_provider.side_effect = Exception("no from_provider") + mock_instructor.Mode.JSON = "json" + mock_instructor.Mode.TOOLS = "tools" + + with patch("semantica.semantic_extract.providers.instructor", mock_instructor): + result = provider.generate_typed("classify this", Schema) + + # Must have called from_openai with the existing client (not a fresh one) + mock_instructor.from_openai.assert_called_once_with( + mock_openai_client, mode="json" + ) + self.assertEqual(result.label, "ok") + + +if __name__ == "__main__": + unittest.main()