From 4dea295f0d0ba2e049347ebbf54e0bacde5ba2ea Mon Sep 17 00:00:00 2001 From: Sameer6305 Date: Fri, 31 Jul 2026 16:05:37 +0530 Subject: [PATCH] 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 --- semantica/conflicts/source_tracker.py | 12 ++++++------ semantica/split/provenance_tracker.py | 12 ++++++++---- tests/conflicts/test_conflicts.py | 2 +- tests/provenance/test_backward_compat.py | 16 ++++++++++++++++ 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/semantica/conflicts/source_tracker.py b/semantica/conflicts/source_tracker.py index d2035c66..9ae19aa9 100644 --- a/semantica/conflicts/source_tracker.py +++ b/semantica/conflicts/source_tracker.py @@ -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: diff --git a/semantica/split/provenance_tracker.py b/semantica/split/provenance_tracker.py index e5d0edb4..e4cd2096 100644 --- a/semantica/split/provenance_tracker.py +++ b/semantica/split/provenance_tracker.py @@ -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) diff --git a/tests/conflicts/test_conflicts.py b/tests/conflicts/test_conflicts.py index 8018cb4c..34dad4d2 100644 --- a/tests/conflicts/test_conflicts.py +++ b/tests/conflicts/test_conflicts.py @@ -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) diff --git a/tests/provenance/test_backward_compat.py b/tests/provenance/test_backward_compat.py index 16268aeb..cb785e5b 100644 --- a/tests/provenance/test_backward_compat.py +++ b/tests/provenance/test_backward_compat.py @@ -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."""