From 37c890bee267181fd15096d881c502c01b7ddc5a Mon Sep 17 00:00:00 2001 From: Sameer6305 Date: Wed, 15 Jul 2026 14:06:54 +0530 Subject: [PATCH 1/3] Fix track_entity re-track silently overriding explicit parent_entity_id/derived_from (fixes #742) --- semantica/provenance/manager.py | 16 ++++++++++++++-- tests/provenance/test_manager.py | 25 +++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/semantica/provenance/manager.py b/semantica/provenance/manager.py index 278a64d0..9bfea349 100644 --- a/semantica/provenance/manager.py +++ b/semantica/provenance/manager.py @@ -128,6 +128,13 @@ class ProvenanceManager: except Exception: pass + # Track whether the caller explicitly supplied a parent link (via + # parent_entity_id kwarg 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 if existing: # Create a history entry for the previous state @@ -145,8 +152,13 @@ class ProvenanceManager: # Store the history entry try: self.storage.store(history_entry) - # Link new entry to this history entry - parent_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) diff --git a/tests/provenance/test_manager.py b/tests/provenance/test_manager.py index 99040fad..a651bbab 100644 --- a/tests/provenance/test_manager.py +++ b/tests/provenance/test_manager.py @@ -154,3 +154,28 @@ 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:") From e81baca5a893ae38cbf1073016e5857edd907fab Mon Sep 17 00:00:00 2001 From: Sameer6305 Date: Wed, 15 Jul 2026 14:28:13 +0530 Subject: [PATCH 2/3] Address Qodo review: cover derived_from in explicit-parent check, keep archived history entries reachable via used_entities --- semantica/provenance/manager.py | 28 +++++++++++++++++---- tests/provenance/test_manager.py | 43 ++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/semantica/provenance/manager.py b/semantica/provenance/manager.py index 9bfea349..3021242d 100644 --- a/semantica/provenance/manager.py +++ b/semantica/provenance/manager.py @@ -25,6 +25,7 @@ License: MIT """ from typing import Optional, List, Dict, Any +from collections.abc import Mapping from datetime import datetime from .schemas import ProvenanceEntry, SourceReference, PropertySource @@ -115,7 +116,14 @@ class ProvenanceManager: # Check if entity already exists existing = self.storage.retrieve(entity_id) parent_id = kwargs.get("parent_entity_id") - + + # If caller declared an explicit parent via metadata, honor it + # (unless parent_entity_id was already passed directly) + if not parent_id and metadata and isinstance(metadata, Mapping): + derived_from = metadata.get("derived_from") + if derived_from and isinstance(derived_from, str): + parent_id = derived_from + # If source is a known entity, link it as parent (unless parent already set) if not parent_id and source and isinstance(source, str): try: @@ -129,13 +137,14 @@ class ProvenanceManager: pass # Track whether the caller explicitly supplied a parent link (via - # parent_entity_id kwarg 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). + # 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 @@ -152,6 +161,7 @@ class ProvenanceManager: # Store the history entry try: self.storage.store(history_entry) + 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 @@ -175,6 +185,14 @@ 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 even when explicit_parent_supplied is + # True and parent_entity_id points to the caller's explicit parent + # rather than the history pointer. + if archived_history_id: + 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 a651bbab..f3533cb9 100644 --- a/tests/provenance/test_manager.py +++ b/tests/provenance/test_manager.py @@ -179,3 +179,46 @@ class TestProvenanceManager: 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" + ) From 869083e0f661b246a2dd2670edf9f98c45824d07 Mon Sep 17 00:00:00 2001 From: KaifAhmad1 Date: Wed, 15 Jul 2026 15:36:48 +0530 Subject: [PATCH 3/3] Avoid duplicating archived history id in used_entities when no explicit parent was supplied; add CHANGELOG entry for #742 Review follow-up: only append archived_history_id to used_entities when explicit_parent_supplied is True. Previously it was appended unconditionally, so the no-explicit-parent re-track path ended up with the same history id in both parent_entity_id and used_entities, duplicating the reference in get_lineage() output. --- CHANGELOG.md | 6 ++++++ semantica/provenance/manager.py | 10 ++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c158e27a..b7528d5f 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 + - **`Reasoner.add_rule` had no deduplication, doubling rules and silently emptying `forward_chain()` on rerun** (#732) by @KaifAhmad1 - `add_rule()` unconditionally appended to `self.rules`, so re-running the same setup code on an existing `Reasoner` instance (e.g. re-executing a Jupyter cell) duplicated every rule; since `forward_chain()` only records a conclusion if it isn't already in `self.facts`, the second run's duplicated rules matched but produced no new results, with no error or warning - `add_rule()` now compares an incoming rule's `rule_type`, `conditions`, and `conclusion` against existing rules and returns the existing `Rule` instead of appending a duplicate, keeping repeated `add_rule()` calls with the same definition idempotent diff --git a/semantica/provenance/manager.py b/semantica/provenance/manager.py index 3021242d..5132260f 100644 --- a/semantica/provenance/manager.py +++ b/semantica/provenance/manager.py @@ -188,10 +188,12 @@ class ProvenanceManager: # 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 even when explicit_parent_supplied is - # True and parent_entity_id points to the caller's explicit parent - # rather than the history pointer. - if archived_history_id: + # 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