mirror of
https://github.com/semantica-agi/semantica.git
synced 2026-09-12 04:01:35 +00:00
fixed qodo reviews
- split.ProvenanceTracker: Check _unified_manager.track_chunk() return value and fall back to legacy storage when None is returned (storage failure) - split.ProvenanceTracker: Initialize legacy stores (_provenance_store and _chunk_registry) unconditionally in __init__ so legacy fallback storage works safely even when initialized with use_unified=True - split.ProvenanceTracker: Update get_provenance() to check legacy storage when no record is found in unified storage, ensuring fallback-tracked chunks remain retrievable - SourceTracker: Change track_sources_batch() condition from if entry is not None: to if ok: to respect the boolean return contract (-> bool) of track_entity_source(), track_property_source(), and track_relationship_source() - Tests: Add regression test test_unified_tracking_returns_none_triggers_fallback and update test_track_sources_batch_failure_not_counted to test boolean failure return_value=False
This commit is contained in:
@@ -614,8 +614,8 @@ class SourceTracker:
|
||||
if item_type == "entity":
|
||||
entity_id = item.get("entity_id")
|
||||
if entity_id:
|
||||
entry = self.track_entity_source(entity_id, source_ref, **metadata)
|
||||
if entry is not None:
|
||||
ok = self.track_entity_source(entity_id, source_ref, **metadata)
|
||||
if ok:
|
||||
stats["entities_tracked"] += 1
|
||||
stats["total_tracked"] += 1
|
||||
else:
|
||||
@@ -628,10 +628,10 @@ class SourceTracker:
|
||||
property_name = item.get("property_name")
|
||||
value = item.get("value")
|
||||
if entity_id and property_name is not None:
|
||||
entry = self.track_property_source(
|
||||
ok = self.track_property_source(
|
||||
entity_id, property_name, value, source_ref, **metadata
|
||||
)
|
||||
if entry is not None:
|
||||
if ok:
|
||||
stats["properties_tracked"] += 1
|
||||
stats["total_tracked"] += 1
|
||||
else:
|
||||
@@ -642,10 +642,10 @@ class SourceTracker:
|
||||
elif item_type == "relationship":
|
||||
relationship_id = item.get("relationship_id")
|
||||
if relationship_id:
|
||||
entry = self.track_relationship_source(
|
||||
ok = self.track_relationship_source(
|
||||
relationship_id, source_ref, **metadata
|
||||
)
|
||||
if entry is not None:
|
||||
if ok:
|
||||
stats["relationships_tracked"] += 1
|
||||
stats["total_tracked"] += 1
|
||||
else:
|
||||
|
||||
@@ -97,6 +97,9 @@ class ProvenanceTracker:
|
||||
self.store_metadata = config.get("store_metadata", True)
|
||||
self.track_versions = config.get("track_versions", False)
|
||||
|
||||
self._provenance_store: Dict[str, ProvenanceInfo] = {}
|
||||
self._chunk_registry: Dict[str, str] = {} # chunk_id -> provenance_id
|
||||
|
||||
# Determine whether to use unified backend
|
||||
use_unified = config.get("use_unified", True) and UNIFIED_AVAILABLE
|
||||
|
||||
@@ -110,8 +113,6 @@ class ProvenanceTracker:
|
||||
# Fallback to legacy in-memory storage
|
||||
self._unified_manager = None
|
||||
self._use_unified = False
|
||||
self._provenance_store: Dict[str, ProvenanceInfo] = {}
|
||||
self._chunk_registry: Dict[str, str] = {} # chunk_id -> provenance_id
|
||||
self.logger.debug("Chunk provenance tracker initialized with legacy backend")
|
||||
|
||||
def track_chunk(
|
||||
@@ -147,7 +148,7 @@ class ProvenanceTracker:
|
||||
# Delegate to unified manager
|
||||
try:
|
||||
chunk_metadata = {**chunk.metadata, **metadata, "chunk_size": len(chunk.text)} if self.store_metadata else metadata
|
||||
self._unified_manager.track_chunk(
|
||||
entry = self._unified_manager.track_chunk(
|
||||
chunk_id=chunk_id,
|
||||
source_document=source_document,
|
||||
source_path=source_path,
|
||||
@@ -156,6 +157,9 @@ class ProvenanceTracker:
|
||||
parent_chunk_id=parent_chunk_id,
|
||||
**chunk_metadata
|
||||
)
|
||||
if entry is None:
|
||||
self.logger.warning("Unified tracking failed (returned None), using fallback")
|
||||
return self._track_chunk_legacy(chunk, source_document, source_path, parent_chunk_id, **metadata)
|
||||
return chunk_id # Return chunk_id for compatibility
|
||||
except Exception as e:
|
||||
self.logger.warning(f"Unified tracking failed, using fallback: {e}")
|
||||
@@ -303,7 +307,7 @@ class ProvenanceTracker:
|
||||
version=prov.get("version", "1.0"),
|
||||
timestamp=prov.get("timestamp")
|
||||
)
|
||||
return None
|
||||
return self._get_provenance_legacy(chunk_id)
|
||||
except Exception as e:
|
||||
self.logger.warning(f"Unified retrieval failed, using fallback: {e}")
|
||||
return self._get_provenance_legacy(chunk_id)
|
||||
|
||||
@@ -103,7 +103,7 @@ class TestConflictsModule(unittest.TestCase):
|
||||
"source": self.source2,
|
||||
},
|
||||
]
|
||||
with patch.object(tracker, "track_property_source", return_value=None):
|
||||
with patch.object(tracker, "track_property_source", return_value=False):
|
||||
stats = tracker.track_sources_batch(source_data)
|
||||
self.assertEqual(stats["properties_tracked"], 0)
|
||||
self.assertEqual(stats["total_tracked"], 0)
|
||||
|
||||
@@ -141,6 +141,22 @@ class TestSplitProvenanceBackwardCompat:
|
||||
|
||||
assert len(prov_ids) == 3
|
||||
|
||||
def test_unified_tracking_returns_none_triggers_fallback(self):
|
||||
"""Test that when _unified_manager.track_chunk returns None, fallback provenance is stored (#783)."""
|
||||
from unittest.mock import patch
|
||||
tracker = SplitProvenanceTracker()
|
||||
chunk = Chunk(text="Test chunk for none fallback", start_index=0, end_index=10, metadata={})
|
||||
|
||||
with patch.object(tracker._unified_manager, "track_chunk", return_value=None):
|
||||
prov_id = tracker.track_chunk(chunk, source_document="doc_1")
|
||||
assert prov_id is not None
|
||||
# Confirm it fell back to legacy store and is retrievable
|
||||
assert chunk.id in tracker._chunk_registry
|
||||
assert prov_id in tracker._provenance_store
|
||||
prov_info = tracker.get_provenance(chunk.id)
|
||||
assert prov_info is not None
|
||||
assert prov_info.source_document == "doc_1"
|
||||
|
||||
|
||||
class TestNoProvenanceOverhead:
|
||||
"""Test that provenance has zero overhead when not used."""
|
||||
|
||||
Reference in New Issue
Block a user