diff --git a/CHANGELOG.md b/CHANGELOG.md index 64221520..15af8917 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **`ProvenanceManager.track_entity` silently overrides an explicit `parent_entity_id`/`derived_from` on re-track** (#742) by @Sameer6305 + - `track_entity()` resolved `parent_id` via a documented precedence chain (`parent_entity_id` kwarg > `metadata["derived_from"]` > source-as-known-entity-id fallback), but the history-preservation block that runs afterward unconditionally overwrote that resolved value with an auto-generated `f"{entity_id}:v:{existing.last_updated}"` history pointer whenever the entity was being re-tracked, discarding whatever parent the caller had just explicitly supplied with no warning + - `track_entity()` now records whether the precedence chain already resolved an explicit parent (`parent_entity_id` kwarg, `metadata["derived_from"]`, or the source-as-known-entity-id fallback) before the history block runs, and only falls back to the auto-generated history pointer when the caller supplied no explicit parent on that call + - The archived history entry for the previous version is still kept reachable in `get_lineage()` via `used_entities` (BFS-traversed by `InMemoryStorage.trace_lineage()`) even when an explicit parent is supplied, so re-tracking with a new parent no longer orphans the prior version from the lineage chain; when no explicit parent is supplied, `used_entities` is left alone since `parent_entity_id` already points at the same history id, avoiding a duplicate self-reference + - Added `test_retrack_with_explicit_parent_overrides_history_link`, `test_retrack_without_explicit_parent_still_uses_history_link`, `test_retrack_with_derived_from_overrides_history_link`, and `test_retrack_history_reachable_via_used_entities` regression tests, closing #742 + - **`ProvenanceManager.get_lineage` does not link entities that share a source URL** (#735) by @KaifAhmad1 - `track_entity()`'s only auto-linking logic looked up `source` as if it were an existing entity's `entity_id`, so passing the same real URL/DOI as `source` for two conceptually linked entities (e.g. a document and a decision derived from it) never produced a parent link, leaving `get_lineage()` returning a chain of length 1 - `metadata["derived_from"]` was preserved and echoed back in the output JSON but was never consulted by any linking or traversal code, so the caller's explicit relationship was silently inert diff --git a/semantica/provenance/manager.py b/semantica/provenance/manager.py index 94b2f622..7d81eed0 100644 --- a/semantica/provenance/manager.py +++ b/semantica/provenance/manager.py @@ -136,7 +136,15 @@ class ProvenanceManager: except Exception: pass + # Track whether the caller explicitly supplied a parent link (via + # parent_entity_id kwarg, metadata["derived_from"], or source-as- + # known-entity-id resolution) BEFORE the history-preservation block + # below. If they did, that explicit value should not be silently + # overwritten by the auto-generated history pointer (#742). + explicit_parent_supplied = parent_id is not None + # If entity exists, preserve history by archiving the old state + archived_history_id = None if existing: # Create a history entry for the previous state # Use timestamp or counter for uniqueness @@ -153,8 +161,14 @@ class ProvenanceManager: # Store the history entry try: self.storage.store(history_entry) - # Link new entry to this history entry - parent_id = history_id + archived_history_id = history_id + # Link new entry to this history entry — but only when the + # caller didn't explicitly supply a new parent on this call. + # An explicit parent_entity_id (or derived_from on branches + # that support it) is an intentional override signal and must + # not be silently replaced by internal bookkeeping (#742). + if not explicit_parent_supplied: + parent_id = history_id except Exception: pass # If history archiving fails, proceed with update but lose history (graceful degradation) @@ -171,6 +185,16 @@ class ProvenanceManager: last_updated=datetime.utcnow().isoformat(), parent_entity_id=parent_id # Link to history or explicit parent ) + + # Make the archived history entry discoverable via trace_lineage()'s + # BFS over used_entities — this ensures the previous version remains + # reachable in the lineage chain when explicit_parent_supplied is True + # and parent_entity_id points to the caller's explicit parent rather + # than the history pointer. When no explicit parent was supplied, + # parent_entity_id already IS archived_history_id, so appending it + # here too would duplicate the same id in both fields (#742 follow-up). + if archived_history_id and explicit_parent_supplied: + entry.used_entities.append(archived_history_id) # Compute checksum for integrity entry.checksum = compute_checksum(entry) diff --git a/tests/provenance/test_manager.py b/tests/provenance/test_manager.py index ea0187ae..6d704d8f 100644 --- a/tests/provenance/test_manager.py +++ b/tests/provenance/test_manager.py @@ -339,3 +339,71 @@ class TestProvenanceManager: lineage = prov_mgr.get_lineage("entity_1") assert lineage == {} + + def test_retrack_with_explicit_parent_overrides_history_link(self): + """#742 — re-tracking an entity with an explicit parent_entity_id must + honor the new value, not silently replace it with an auto-generated + history pointer.""" + prov_mgr = ProvenanceManager() + + e1 = prov_mgr.track_entity("X", source="doc_1", parent_entity_id="parent_v1") + e2 = prov_mgr.track_entity("X", source="doc_1", parent_entity_id="parent_v2") + + assert e1.parent_entity_id == "parent_v1" + assert e2.parent_entity_id == "parent_v2" + + def test_retrack_without_explicit_parent_still_uses_history_link(self): + """#742 — when NO explicit parent is given on a re-track call, the + auto-generated history link (Y:v:) should still be used, + preserving pre-existing behavior for callers that don't supply a parent.""" + prov_mgr = ProvenanceManager() + + y1 = prov_mgr.track_entity("Y", source="doc_1") + y2 = prov_mgr.track_entity("Y", source="doc_1") + + assert y1.parent_entity_id is None + assert y2.parent_entity_id is not None + assert y2.parent_entity_id.startswith("Y:v:") + + def test_retrack_with_derived_from_overrides_history_link(self): + """#742 — re-tracking with metadata['derived_from'] (no parent_entity_id + kwarg) should also override the auto-generated history link, not just + the parent_entity_id kwarg case.""" + prov_mgr = ProvenanceManager() + + prov_mgr.track_entity("parent_A", source="doc_1") + prov_mgr.track_entity("parent_B", source="doc_1") + + e1 = prov_mgr.track_entity("Z", source="doc_1", metadata={"derived_from": "parent_A"}) + e2 = prov_mgr.track_entity("Z", source="doc_1", metadata={"derived_from": "parent_B"}) + + assert e1.parent_entity_id == "parent_A" + assert e2.parent_entity_id == "parent_B" + + def test_retrack_history_reachable_via_used_entities(self): + """#742 — when re-tracking with an explicit parent, the archived history + entry for the previous version must still be reachable in the lineage + chain via used_entities (prov:used), even though it's no longer the + direct parent_entity_id.""" + prov_mgr = ProvenanceManager() + + prov_mgr.track_entity("explicit_parent", source="doc_1") + prov_mgr.track_entity("X", source="doc_1") # first track, no parent + + # Re-track with an explicit parent — should NOT lose the history entry + e2 = prov_mgr.track_entity("X", source="doc_1", parent_entity_id="explicit_parent") + + assert e2.parent_entity_id == "explicit_parent" + assert len(e2.used_entities) == 1 + assert e2.used_entities[0].startswith("X:v:") + + # trace_lineage should reach: X, explicit_parent (via parent_entity_id), + # AND the archived history snapshot (via used_entities) + lineage = prov_mgr.get_lineage("X") + entity_ids = {e["entity_id"] for e in lineage["lineage_chain"]} + + assert "X" in entity_ids + assert "explicit_parent" in entity_ids + assert e2.used_entities[0] in entity_ids, ( + "Archived history entry should be reachable via used_entities in lineage" + )