fix(kg): align GraphBuilder raw-text extraction defaults with the documented contract (#941)

* fix(kg): align GraphBuilder raw-text extraction defaults with the documented contract

_extract_from_text() defaulted ner_method, relation_method and
triplet_method to "llm" and ran relation extraction unconditionally,
contradicting the build() docstring ("ml"/"pattern"/False) and the
standalone extractor defaults. Any raw-text build() therefore required a
provider, an API key, and network access without saying so.

Defaults are now ml/pattern/pattern with extract_relations=False. LLM
extraction is unchanged and now opt-in via explicit kwargs.

Also documents relation_method and extract_triplets, which the docstring
never listed, and drops the stale "Default to LLM methods as per
requirement" comment.

Closes #930

* perf(kg): reuse extractors across texts instead of rebuilding per source

Addresses review feedback on #941. NERExtractor.__init__ loads its spaCy
model eagerly when the method includes "ml", so switching the default
from "llm" to "ml" made _extract_from_text() reload the model once per
source in a multi-document build.

Extractors are now cached per (kind, method) on the builder. Adds tests
asserting single construction across repeated texts, that distinct
methods still get distinct extractors, and that the default path runs
end to end without any provider call.

* fix(kg): keep fallback method lists working with the extractor cache

The extractor cache keyed directly on `method`, but all three extractors
accept a list for fallback ordering (e.g. ner_method=["pattern", "ml"]),
so a list argument raised TypeError: unhashable type: 'list' before
extraction started. Lists are now converted to tuples for the cache key
only; the extractor still receives the original value.

Also seeds _extraction_stats in __init__. It was previously created only
in build(), so calling _extract_from_text() directly — as the report's
repro does — raised an AttributeError that the broad except swallowed and
logged as "Entity extraction failed".

Adds coverage for list methods on all three extractors, cache reuse for
equal lists, and distinct entries for different orderings.

* fix(kg): forward extracted relations into triplet extraction

_extract_from_text() passed only entities= to extract_triplets(), so
TripletExtractor re-derived relations itself whenever relations is None,
using a method taken from triplet_method rather than relation_method.
That duplicated work and could yield triplets inconsistent with the
relations already extracted.

relations is now initialized to None, holds the extracted list when
extract_relations=True succeeds, and is forwarded to extract_triplets().
When extraction is disabled or fails, None is passed and
TripletExtractor's existing self-derivation is unchanged.

Folded in at maintainer request rather than tracked as #944.

* docs(changelog): note that #878 documented the LLM defaults before this landed

#878 merged while this was in review and resolved the same code/docstring
mismatch in the opposite direction. Records that #930's decision makes
the code the side that changes, and that #878's docstring formatting is
retained.

CHANGELOG.md

@@ -18,6 +18,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - **Corrected during review**: `add_temporal_edge`/`create_temporal_snapshot` docstrings overclaimed numeric-timestamp support; `_parse_time()` only special-cases `str` and `datetime`, falling back to a bare `str()` cast for anything else (not true numeric parsing). Narrowed to "datetime or ISO-formatted string"
   - **Fixed along the way**: `build()`'s `**options` documented a default only for `extract`; `extract_relations`, `extract_triplets`, `ner_method`, `relation_method`, and `triplet_method` all have concrete defaults in `_extract_from_text()` (`True`, `True`, `"llm"`, `"llm"`, `"llm"`) that were left unstated, inconsistent with CONTRIBUTING.md's own docstring example of noting defaults inline
   - `python -m pytest tests/kg/test_kg.py tests/kg/test_graph_builder_external.py -q`: 45 passed
+- **`GraphBuilder` raw-text extraction now defaults to local extractors instead of LLM extraction** (closes #930) by @dex0shubham
+  - `GraphBuilder._extract_from_text()` defaulted `ner_method`, `relation_method`, and `triplet_method` to `"llm"`, and ran relation extraction unconditionally (`extract_relations` defaulted to `True`) — all four contradicting the defaults documented in the `build()` docstring at the time (`"ml"` / `"pattern"` / `False`), and diverging from the standalone extractors (`NERExtractor` defaults to `method="ml"`, `RelationExtractor` and `TripletExtractor` to `method="pattern"`). The practical effect was that any raw-text `build()` call silently required a configured provider, an API key, and network access
+  - Defaults are now `ner_method="ml"`, `relation_method="pattern"`, `triplet_method="pattern"`, and `extract_relations=False`, matching the docstring. LLM extraction remains fully available and is now opt-in
+  - **To restore the previous behaviour**, pass the methods explicitly:
+    ```python
+    builder.build(
+        sources,
+        ner_method="llm",
+        relation_method="llm",
+        triplet_method="llm",
+        extract_relations=True,
+    )
+    ```
+  - #878 landed in the meantime and resolved the same mismatch in the opposite direction, documenting the LLM values (`"llm"` / `"llm"` / `"llm"`, `extract_relations: True`) as the contract. Per the decision on #930 the code is the side that changes, so those docstring defaults are corrected here to `"ml"` / `"pattern"` / `"pattern"` / `False`, keeping #878's formatting
+  - Removed the stale `# Default to LLM methods as per requirement` comment, which read as an intentional decision but did not match the documented contract
+  - **Fixed along the way**: `_extract_from_text()` constructed a fresh extractor for every text, and `NERExtractor.__init__` loads its spaCy model eagerly when the method includes `"ml"` — so with the new default, a multi-document build would have reloaded the model once per source. Extractors are now built once per `(kind, method)` and reused for the lifetime of the builder, via `GraphBuilder._get_extractor()`. This path was previously unreachable by default because the old `"llm"` default never touched spaCy
+  - **Fixed along the way**: `_extract_from_text()` never forwarded its extracted relations to triplet extraction — it passed only `entities=`, so `TripletExtractor` re-derived relations itself (via a method taken from `triplet_method`) whenever `relations is None`, duplicating work and producing triplets that could disagree with the relations already extracted using `relation_method`. Relations are now passed through as `relations=`; when relation extraction is disabled or fails, `None` is forwarded and `TripletExtractor` keeps its existing self-derivation behaviour
+  - **Fixed along the way**: `GraphBuilder._extraction_stats` was only initialised inside `build()`, so calling `_extract_from_text()` directly raised an `AttributeError` that the extraction path's broad `except` swallowed and reported as `"Entity extraction failed"`. It is now seeded in `__init__` as well; `build()` still resets it per run
+  - New regression coverage in `tests/kg/test_graph_builder_extraction_defaults.py` pinning all four defaults, verifying that no default resolves to `"llm"`, confirming explicit LLM opt-in still routes correctly, asserting extractors are constructed once across repeated texts, covering fallback method lists (e.g. `ner_method=["pattern", "ml"]`) for all three extractors, asserting relations are forwarded to triplet extraction (and that `None` is forwarded when relation extraction is disabled or fails), and running the real default path end to end with no provider mocked. Verified to fail against the pre-fix code
+  - Full `kg` suite: 473 passed
 
 ### Fixed
 

semantica/kg/graph_builder.py

@@ -23,7 +23,7 @@ License: MIT
 """
 
 from datetime import datetime
-from typing import Any, Dict, List, Optional, Union
+from typing import Any, Dict, List, Optional, Tuple, Union
 import time
 
 
@@ -90,6 +90,18 @@ class GraphBuilder:
         self.version_snapshots = version_snapshots
         self.graph_store = graph_store
         self.config = kwargs  # Store additional config for extractors
+        # Extractors are reused across texts: NERExtractor loads its spaCy model
+        # eagerly in __init__, so constructing one per text would reload the
+        # model on every source in a multi-document build.
+        self._extractor_cache: Dict[Tuple[str, Any], Any] = {}
+        # build() resets these per run; seed them here so _extract_from_text
+        # is usable on its own instead of raising an AttributeError that the
+        # broad except in the extraction path silently swallows.
+        self._extraction_stats: Dict[str, int] = {
+            "extracted_entities": 0,
+            "extracted_relations": 0,
+            "extracted_triplets": 0,
+        }
 
         # Initialize logging
         from ..utils.logging import get_logger
@@ -228,6 +240,28 @@ class GraphBuilder:
             # Unknown type
             pass
 
+    def _get_extractor(
+        self, kind: str, extractor_cls, method: Union[str, List[str]]
+    ):
+        """Return a cached extractor for this method, building it on first use.
+
+        Extractors hold no per-text state but are expensive to construct —
+        ``NERExtractor(method="ml")`` loads a spaCy model in ``__init__``.
+        Keying on kind and method is enough because ``self.config`` is fixed
+        for the lifetime of the builder.
+
+        Args:
+            kind: Extractor role, one of ``"ner"``, ``"relation"``, ``"triplet"``.
+            extractor_cls: Extractor class to construct on a cache miss.
+            method: A method name, or a list of them for fallback ordering.
+                Lists are converted to tuples for the cache key only; the
+                extractor still receives the original value.
+        """
+        key = (kind, tuple(method) if isinstance(method, list) else method)
+        if key not in self._extractor_cache:
+            self._extractor_cache[key] = extractor_cls(method=method, **self.config)
+        return self._extractor_cache[key]
+
     def _extract_from_text(self, text: str, all_entities: List[Any], all_relationships: List[Any], **options):
         """Helper to extract knowledge from text using configured methods."""
         if not options.get("extract", True):
@@ -237,15 +271,17 @@ class GraphBuilder:
         from ..semantic_extract.relation_extractor import RelationExtractor
         from ..semantic_extract.triplet_extractor import TripletExtractor
         
-        # Default to LLM methods as per requirement
-        ner_method = options.get("ner_method", "llm")
-        relation_method = options.get("relation_method", "llm")
-        triplet_method = options.get("triplet_method", "llm")
+        # Local extractors by default — raw-text build() must not require a
+        # provider, API key, or network access. Pass ner_method="llm" (and the
+        # relation/triplet equivalents) to opt into LLM extraction.
+        ner_method = options.get("ner_method", "ml")
+        relation_method = options.get("relation_method", "pattern")
+        triplet_method = options.get("triplet_method", "pattern")
         
         self.logger.info(f"Extracting knowledge from text ({len(text)} chars) using {ner_method}...")
         
         # 1. Extract Entities
-        ner = NERExtractor(method=ner_method, **self.config)
+        ner = self._get_extractor("ner", NERExtractor, ner_method)
         try:
             entities = ner.extract_entities(text, **options)
             extracted_count = len(entities)
@@ -258,8 +294,16 @@ class GraphBuilder:
             entities = []
         
         # 2. Extract Relations (if requested)
-        if options.get("extract_relations", True):
-            rel_extractor = RelationExtractor(method=relation_method, **self.config)
+        # Stays None when relation extraction is skipped or fails, which lets
+        # TripletExtractor derive its own relations as before. When we do have
+        # them, they are forwarded below so triplets reuse the relations
+        # extracted with relation_method rather than re-deriving via
+        # triplet_method.
+        relations = None
+        if options.get("extract_relations", False):
+            rel_extractor = self._get_extractor(
+                "relation", RelationExtractor, relation_method
+            )
             try:
                 # Pass entities if available to help relation extraction
                 relations = rel_extractor.extract_relations(text, entities=entities, **options)
@@ -273,9 +317,13 @@ class GraphBuilder:
 
         # 3. Extract Triplets (if requested)
         if options.get("extract_triplets", True):
-            trip_extractor = TripletExtractor(method=triplet_method, **self.config)
+            trip_extractor = self._get_extractor(
+                "triplet", TripletExtractor, triplet_method
+            )
             try:
-                triplets = trip_extractor.extract_triplets(text, entities=entities, **options)
+                triplets = trip_extractor.extract_triplets(
+                    text, entities=entities, relations=relations, **options
+                )
                 extracted_count = len(triplets)
                 self._extraction_stats["extracted_triplets"] += extracted_count
                 self.logger.info(f"Extracted {extracted_count} triplets")
@@ -307,20 +355,24 @@ class GraphBuilder:
                   raw string or ``{"text": ...}`` dict is passed as a source
                   (default: ``True``).
                 - ``extract_relations`` (bool): Whether to extract relations
-                  during text extraction (default: ``True``).
+                  during text extraction (default: ``False``).
                 - ``extract_triplets`` (bool): Whether to extract triplets
                   during text extraction (default: ``True``).
                 - ``ner_method`` (str): NER backend used for text extraction
-                  (e.g. ``"ml"``, ``"pattern"``, ``"llm"``; default: ``"llm"``).
+                  (e.g. ``"ml"``, ``"pattern"``, ``"llm"``; default: ``"ml"``).
                 - ``relation_method`` (str): Relation-extraction backend
-                  (e.g. ``"pattern"``, ``"llm"``; default: ``"llm"``).
+                  (e.g. ``"pattern"``, ``"llm"``; default: ``"pattern"``).
                 - ``triplet_method`` (str): Triplet-extraction backend
-                  (e.g. ``"pattern"``, ``"llm"``; default: ``"llm"``).
+                  (e.g. ``"pattern"``, ``"llm"``; default: ``"pattern"``).
                 - ``entity_resolver``: An :class:`EntityResolver` instance
                   that overrides the one configured on the builder.
                 - ``relationships`` (list): An explicit list of relationships
                   to include in addition to those found in *sources*.
 
+            Raw-text extraction uses local extractors by default and needs no
+            provider or API key. To use LLM extraction, pass the methods
+            explicitly, e.g. ``ner_method="llm"``.
+
         Returns:
             A dictionary containing the graph's ``entities``,
             ``relationships``, and build ``metadata``.

tests/kg/test_graph_builder_extraction_defaults.py

@@ -0,0 +1,262 @@
+"""Pins GraphBuilder's raw-text extraction defaults to the documented values.
+
+Regression guard for #930: `_extract_from_text` defaulted to LLM extraction for
+all three methods and ran relation extraction unconditionally, both of which
+contradicted the `build()` docstring and silently required a provider and API
+key for any raw-text build.
+"""
+
+import unittest
+from unittest.mock import patch
+
+from semantica.kg.graph_builder import GraphBuilder
+
+
+class TestGraphBuilderExtractionDefaults(unittest.TestCase):
+
+    def setUp(self):
+        self.ner_patcher = patch(
+            "semantica.semantic_extract.ner_extractor.NERExtractor"
+        )
+        self.rel_patcher = patch(
+            "semantica.semantic_extract.relation_extractor.RelationExtractor"
+        )
+        self.trip_patcher = patch(
+            "semantica.semantic_extract.triplet_extractor.TripletExtractor"
+        )
+        self.NER = self.ner_patcher.start()
+        self.Rel = self.rel_patcher.start()
+        self.Trip = self.trip_patcher.start()
+        self.addCleanup(self.ner_patcher.stop)
+        self.addCleanup(self.rel_patcher.stop)
+        self.addCleanup(self.trip_patcher.stop)
+
+        self.NER.return_value.extract_entities.return_value = []
+        self.Rel.return_value.extract_relations.return_value = []
+        self.Trip.return_value.extract_triplets.return_value = []
+
+        self.builder = GraphBuilder(merge_entities=False, resolve_conflicts=False)
+
+    def _extract(self, **options):
+        self.builder._extract_from_text(
+            "Apple Inc. was founded in 1976.", [], [], **options
+        )
+
+    def test_ner_method_defaults_to_ml(self):
+        self._extract()
+        self.assertEqual(self.NER.call_args.kwargs["method"], "ml")
+
+    def test_triplet_method_defaults_to_pattern(self):
+        self._extract()
+        self.assertEqual(self.Trip.call_args.kwargs["method"], "pattern")
+
+    def test_relation_extraction_is_off_by_default(self):
+        self._extract()
+        self.Rel.assert_not_called()
+
+    def test_relation_method_defaults_to_pattern_when_enabled(self):
+        self._extract(extract_relations=True)
+        self.assertEqual(self.Rel.call_args.kwargs["method"], "pattern")
+
+    def test_no_extractor_defaults_to_llm(self):
+        """No raw-text default may require a provider or API key."""
+        self._extract(extract_relations=True)
+        extractors = (
+            ("ner", self.NER),
+            ("relation", self.Rel),
+            ("triplet", self.Trip),
+        )
+        for name, mock_cls in extractors:
+            with self.subTest(extractor=name):
+                self.assertNotEqual(mock_cls.call_args.kwargs["method"], "llm")
+
+    def test_llm_extraction_is_still_available_explicitly(self):
+        self._extract(
+            ner_method="llm",
+            relation_method="llm",
+            triplet_method="llm",
+            extract_relations=True,
+        )
+        self.assertEqual(self.NER.call_args.kwargs["method"], "llm")
+        self.assertEqual(self.Rel.call_args.kwargs["method"], "llm")
+        self.assertEqual(self.Trip.call_args.kwargs["method"], "llm")
+
+
+class TestGraphBuilderExtractorReuse(unittest.TestCase):
+    """Extractors must be built once per method, not once per text.
+
+    `NERExtractor.__init__` loads its spaCy model eagerly, so with the `"ml"`
+    default a per-text construction would reload the model for every source in
+    a multi-document build.
+    """
+
+    def setUp(self):
+        self.ner_patcher = patch(
+            "semantica.semantic_extract.ner_extractor.NERExtractor"
+        )
+        self.NER = self.ner_patcher.start()
+        self.addCleanup(self.ner_patcher.stop)
+        self.NER.return_value.extract_entities.return_value = []
+
+        self.builder = GraphBuilder(merge_entities=False, resolve_conflicts=False)
+
+    def test_ner_extractor_built_once_across_texts(self):
+        for i in range(5):
+            self.builder._extract_from_text(f"Document {i}.", [], [])
+        self.assertEqual(self.NER.call_count, 1)
+
+    def test_distinct_methods_get_distinct_extractors(self):
+        self.builder._extract_from_text("a", [], [])
+        self.builder._extract_from_text("b", [], [], ner_method="pattern")
+        self.builder._extract_from_text("c", [], [])
+        self.assertEqual(self.NER.call_count, 2)
+
+
+class TestGraphBuilderForwardsRelationsToTriplets(unittest.TestCase):
+    """Relations extracted with relation_method must reach triplet extraction.
+
+    `TripletExtractor` re-derives relations itself when `relations is None`,
+    using a method derived from `triplet_method` — so not forwarding them both
+    duplicates work and can produce triplets inconsistent with the relations
+    already extracted.
+    """
+
+    def setUp(self):
+        self.ner_patcher = patch(
+            "semantica.semantic_extract.ner_extractor.NERExtractor"
+        )
+        self.rel_patcher = patch(
+            "semantica.semantic_extract.relation_extractor.RelationExtractor"
+        )
+        self.trip_patcher = patch(
+            "semantica.semantic_extract.triplet_extractor.TripletExtractor"
+        )
+        self.NER = self.ner_patcher.start()
+        self.Rel = self.rel_patcher.start()
+        self.Trip = self.trip_patcher.start()
+        self.addCleanup(self.ner_patcher.stop)
+        self.addCleanup(self.rel_patcher.stop)
+        self.addCleanup(self.trip_patcher.stop)
+
+        self.NER.return_value.extract_entities.return_value = []
+        self.Trip.return_value.extract_triplets.return_value = []
+
+        self.builder = GraphBuilder(merge_entities=False, resolve_conflicts=False)
+
+    def _triplet_kwargs(self):
+        return self.Trip.return_value.extract_triplets.call_args.kwargs
+
+    def test_extracted_relations_are_forwarded(self):
+        sentinel = [object()]
+        self.Rel.return_value.extract_relations.return_value = sentinel
+
+        self.builder._extract_from_text("x", [], [], extract_relations=True)
+
+        self.assertIs(self._triplet_kwargs()["relations"], sentinel)
+
+    def test_relations_is_none_when_extraction_disabled(self):
+        """Default path keeps TripletExtractor's own relation derivation."""
+        self.builder._extract_from_text("x", [], [])
+
+        self.assertIsNone(self._triplet_kwargs()["relations"])
+        self.Rel.assert_not_called()
+
+    def test_relations_is_none_when_extraction_fails(self):
+        self.Rel.return_value.extract_relations.side_effect = RuntimeError("boom")
+
+        self.builder._extract_from_text("x", [], [], extract_relations=True)
+
+        self.assertIsNone(self._triplet_kwargs()["relations"])
+
+
+class TestGraphBuilderFallbackMethodLists(unittest.TestCase):
+    """All three extractors accept a list of methods for fallback ordering.
+
+    The extractor cache must key on something hashable, or passing a list
+    raises `TypeError: unhashable type: 'list'` before extraction even starts.
+    """
+
+    def setUp(self):
+        self.ner_patcher = patch(
+            "semantica.semantic_extract.ner_extractor.NERExtractor"
+        )
+        self.rel_patcher = patch(
+            "semantica.semantic_extract.relation_extractor.RelationExtractor"
+        )
+        self.trip_patcher = patch(
+            "semantica.semantic_extract.triplet_extractor.TripletExtractor"
+        )
+        self.NER = self.ner_patcher.start()
+        self.Rel = self.rel_patcher.start()
+        self.Trip = self.trip_patcher.start()
+        self.addCleanup(self.ner_patcher.stop)
+        self.addCleanup(self.rel_patcher.stop)
+        self.addCleanup(self.trip_patcher.stop)
+
+        self.NER.return_value.extract_entities.return_value = []
+        self.Rel.return_value.extract_relations.return_value = []
+        self.Trip.return_value.extract_triplets.return_value = []
+
+        self.builder = GraphBuilder(merge_entities=False, resolve_conflicts=False)
+
+    def test_list_method_does_not_raise(self):
+        self.builder._extract_from_text(
+            "x", [], [], ner_method=["pattern", "ml"], extract_triplets=False
+        )
+        self.assertEqual(self.NER.call_args.kwargs["method"], ["pattern", "ml"])
+
+    def test_list_methods_accepted_for_every_extractor(self):
+        self.builder._extract_from_text(
+            "x",
+            [],
+            [],
+            ner_method=["pattern", "ml"],
+            relation_method=["pattern", "cooccurrence"],
+            triplet_method=["pattern", "rules"],
+            extract_relations=True,
+        )
+        self.assertEqual(self.NER.call_args.kwargs["method"], ["pattern", "ml"])
+        self.assertEqual(
+            self.Rel.call_args.kwargs["method"], ["pattern", "cooccurrence"]
+        )
+        self.assertEqual(self.Trip.call_args.kwargs["method"], ["pattern", "rules"])
+
+    def test_equal_lists_reuse_one_extractor(self):
+        for _ in range(3):
+            self.builder._extract_from_text(
+                "x", [], [], ner_method=["pattern", "ml"], extract_triplets=False
+            )
+        self.assertEqual(self.NER.call_count, 1)
+
+    def test_different_lists_get_different_extractors(self):
+        self.builder._extract_from_text(
+            "x", [], [], ner_method=["pattern", "ml"], extract_triplets=False
+        )
+        self.builder._extract_from_text(
+            "x", [], [], ner_method=["ml", "pattern"], extract_triplets=False
+        )
+        self.assertEqual(self.NER.call_count, 2)
+
+
+class TestGraphBuilderDefaultsRunOffline(unittest.TestCase):
+    """The default raw-text path must work with no provider and no network."""
+
+    def test_default_build_needs_no_provider(self):
+        builder = GraphBuilder(merge_entities=False, resolve_conflicts=False)
+        entities, relationships = [], []
+
+        # No mocks: this runs the real ml/pattern extractors end to end. If any
+        # default resolved to "llm", this would attempt a provider call.
+        with patch("semantica.semantic_extract.providers.create_provider") as provider:
+            builder._extract_from_text(
+                "Apple Inc. was founded by Steve Jobs in 1976.",
+                entities,
+                relationships,
+            )
+
+        provider.assert_not_called()
+        self.assertIsInstance(entities, list)
+
+
+if __name__ == "__main__":
+    unittest.main()