* fix mutable default argument in graph_analyzer.py
* fix mutable default argument in kg_chunkers.py
* fix mutable default argument in methods.py
* Address review: move default-init code out of docstrings, default levels in split_hierarchical
Three findings from the Qodo review:
- analyze_temporal_evolution: the 'if metrics is None' block had landed
inside the docstring, so it never executed and metrics_tracked came back
None. Moved below the docstring where it runs.
- HierarchicalChunker.__init__: the same misplacement turned the docstring
into a dead string constant and broke help()/introspection. Moved the
default-init below it.
- split_hierarchical: the signature now defaults levels to None, but the
body still ran 'in levels' membership tests — calling it without levels
raised TypeError. Defaults to the documented hierarchy, matching the
class-level default.
* test: add mutable-default regression tests for the three fixed sites
- tests/split/test_chunkers.py: TestMutableDefaultRegression (6 tests)
- split_hierarchical() default levels and chunk_sizes stay independent across calls
- HierarchicalChunker() default levels stay independent across instances
- tests/kg/test_kg.py: TestAnalyzeTemporalEvolutionMutableDefault (5 tests)
- analyze_temporal_evolution() default metrics value is canonical
- mutations to a returned metrics_tracked list do not affect the next call
- explicit metrics override is forwarded and reflected in the return value
- mutating an explicitly passed list does not corrupt a subsequent default call
All 96 tests in the two affected test files pass.
---------
Co-authored-by: Zohaib Hassnain <109234410+ZohaibHassan16@users.noreply.github.com>
Co-authored-by: Sameer Kadam <sskadam6305@gmail.com>
- Reject non-positive stride values before chunking and validate temporary overlap overrides before mutating chunker state.
- Restore the original overlap and custom stride with `try/finally` so state remains unchanged after both successful and failed `chunk_with_overlap()` calls.
- Add regression coverage for invalid stride and overlap values, valid boundary cases, and state restoration.
* test(ner): fix NER configuration tests for the typed LLM extraction API
Two of the three failing tests tracked in #1059 were still red after
#1070 was closed because the mocks targeted the pre-typed provider API:
- test_ner_llm_config mocked generate_structured, but the LLM path now
goes through generate_typed with a Pydantic schema. Mock the typed
response (namespace items with .text/.label/.start/.end/.confidence)
and expect extraction_method 'llm_typed'.
- test_ner_pattern_config asserted 'Apple Inc' without the trailing
dot, but the ORG pattern captures it via (?:\.|\b). Assert 'Apple
Inc.' to match current production behavior.
Verified locally: 8/8 pass in test_ner_configurations.py; the
performance-test failures in tests/semantic_extract/ reproduce on a
clean main checkout and are unrelated.
Fixes#1059
Signed-off-by: Yunare Maia <yunare@gmail.com>
* refactor(ner): remove dead _extract_with_spacy method and unused self.nlp
_extract_with_spacy() had no callers: the ML dispatch path goes through
get_entity_method('ml') -> extract_entities_ml(), which loads the spaCy
model lazily via the process-level cache in methods.py. The instance
attribute self.nlp was only read by that dead method, so __init__ now
just validates the runtime (keeping the _ml_runtime_usable gate) instead
of eagerly loading a model that was never used.
Fixes#1058
Signed-off-by: Yunare Maia <yunare@gmail.com>
* test(split): rewrite NERExtractor cache tests to not rely on removed .nlp attribute
NERExtractor.nlp was removed in this PR as part of dead-code cleanup
(the attribute was only used by the equally-dead _extract_with_spacy()).
The three affected tests in TestNERExtractorSpacyModelCache previously
verified cache behavior through .nlp identity comparisons; rewrite them
to use load-call counts and direct se_methods.load_spacy_model() cache
queries instead:
- test_ner_extractor_reuses_cached_model_across_instances: drop the
e1.nlp is e2.nlp is e3.nlp assertion; len(calls)==1 already proves
reuse; add a cache query to confirm the cached object is non-None.
- test_ner_extractor_distinct_model_names_load_separately: store each
mock nlp in a dict keyed by name, then query the cache to assert
sm_cached is loaded['en_core_web_sm'] and sm_cached is not lg_cached.
- test_ner_extractor_failed_load_not_cached_and_retried: replace
extractor.nlp is None/not None with is-not-None construction checks
and a final cache query that verifies the recovered model is the
exact object returned by working_load.
All three tests still exercise the original behavioral contract (no
crash on missing model, failures not cached / retried, successful load
shared across instances); they just no longer rely on a private
instance attribute that no longer exists.
---------
Signed-off-by: Yunare Maia <yunare@gmail.com>
Co-authored-by: Sameer Kadam <sskadam6305@gmail.com>
SemanticChunker.__init__ only caught OSError around load_spacy_model(),
while NERExtractor's identical call (fixed earlier in this PR) also
catches generic Exception for a model that is installed but fails at
runtime. Bring SemanticChunker in line so a broken spaCy config
degrades to fallback chunking instead of crashing __init__.
Adds a regression test mirroring the existing NERExtractor case, and a
CHANGELOG entry for #998/#1042.
- Robust ID extraction in CentralityCalculator, CommunityDetector, and ConnectivityAnalyzer
- Support for direct Entity objects and dictionaries as node identifiers
- Improved Entity hashability in utils/types.py
- Added integration test to verify fix and prevent regression
