From 7d83b6744fd829e5ed722cb5e522ca5acbb1b831 Mon Sep 17 00:00:00 2001 From: KaifAhmad1 Date: Wed, 15 Jul 2026 12:14:53 +0530 Subject: [PATCH 1/2] Fix ProvenanceManager.get_lineage not linking entities via derived_from track_entity() only auto-linked a parent by looking up `source` as an existing entity_id, so two entities sharing a real source URL (e.g. a document and a decision derived from it) never got connected, and metadata["derived_from"] was stored but never consulted by any linking or traversal code. track_entity() now treats metadata["derived_from"] as an explicit parent link (unless parent_entity_id was already passed directly), so the existing BFS in trace_lineage() picks it up for free. Closes #735 --- CHANGELOG.md | 6 ++ semantica/provenance/manager.py | 9 +- tests/provenance/test_manager.py | 145 ++++++++++++++++++++++++++++++- 3 files changed, 158 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6941b54a..438c4f48 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.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 + - `track_entity()` now treats `metadata["derived_from"]` as an explicit parent link (unless `parent_entity_id` was already passed directly), so `InMemoryStorage.trace_lineage()`'s existing BFS over `parent_entity_id` picks it up for free + - Added 7 regression/edge-case tests in `tests/provenance/test_manager.py` covering the happy path, explicit `parent_entity_id` precedence over `derived_from`, precedence over the `source`-as-known-entity-id fallback, a `derived_from` pointing at a never-tracked entity, non-string/empty-string `derived_from` values being ignored, a self-referencing `derived_from` not hanging traversal, and multi-hop `derived_from` chains, closing #735 + - **`InferenceResult.premises` always empty from `forward_chain`/`backward_chain`** (#739) by @Sameer6305 - `_match_rule()` discarded matched facts and returned only instantiated conclusions, so `ExplanationGenerator` always produced empty premises lists regardless of which facts actually satisfied a rule, closing #733 - `_match_rule()` now returns `(conclusion, matched_facts)` tuples; `forward_chain()` threads those facts into `InferenceResult(premises=...)`, merging premises when the same conclusion is derived more than once within a pass diff --git a/semantica/provenance/manager.py b/semantica/provenance/manager.py index 278a64d0..c9f8bb08 100644 --- a/semantica/provenance/manager.py +++ b/semantica/provenance/manager.py @@ -115,7 +115,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, dict): + 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: diff --git a/tests/provenance/test_manager.py b/tests/provenance/test_manager.py index 99040fad..0b85f73f 100644 --- a/tests/provenance/test_manager.py +++ b/tests/provenance/test_manager.py @@ -104,7 +104,150 @@ class TestProvenanceManager: assert lineage is not None assert "lineage_chain" in lineage assert len(lineage["lineage_chain"]) > 0 - + + def test_get_lineage_via_derived_from_metadata(self): + """metadata['derived_from'] should link entities into the lineage chain + even when they share a source URL rather than one being a known entity_id.""" + prov_mgr = ProvenanceManager() + + prov_mgr.track_entity( + entity_id="doc:X", + source="https://example.com/api", + metadata={"content_type": "drug_label"}, + ) + prov_mgr.track_entity( + entity_id="decision:Y", + source="https://example.com/api", + metadata={"derived_from": "doc:X"}, + ) + + lineage = prov_mgr.get_lineage("decision:Y") + + assert lineage["entity_count"] == 2 + entity_ids = [e["entity_id"] for e in lineage["lineage_chain"]] + assert "doc:X" in entity_ids + assert "decision:Y" in entity_ids + + def test_derived_from_does_not_override_explicit_parent(self): + """An explicit parent_entity_id kwarg should win over metadata['derived_from'].""" + prov_mgr = ProvenanceManager() + + prov_mgr.track_entity(entity_id="explicit_parent", source="doc_1") + prov_mgr.track_entity(entity_id="ignored_parent", source="doc_1") + entry = prov_mgr.track_entity( + entity_id="child", + source="doc_1", + metadata={"derived_from": "ignored_parent"}, + parent_entity_id="explicit_parent", + ) + + assert entry.parent_entity_id == "explicit_parent" + + def test_derived_from_takes_precedence_over_source_as_entity_id(self): + """If `source` happens to also be a known entity_id, an explicit + metadata['derived_from'] should still win over that fallback linking.""" + prov_mgr = ProvenanceManager() + + prov_mgr.track_entity(entity_id="source_as_entity", source="doc_0") + prov_mgr.track_entity(entity_id="real_parent", source="doc_0") + entry = prov_mgr.track_entity( + entity_id="child", + source="source_as_entity", # resolvable as an entity_id + metadata={"derived_from": "real_parent"}, + ) + + assert entry.parent_entity_id == "real_parent" + + def test_derived_from_nonexistent_entity_does_not_crash(self): + """derived_from pointing at an entity that was never tracked should be + stored as the parent link without raising, and lineage traversal should + stop gracefully instead of erroring.""" + prov_mgr = ProvenanceManager() + + entry = prov_mgr.track_entity( + entity_id="orphan_child", + source="doc_1", + metadata={"derived_from": "never_tracked"}, + ) + + assert entry.parent_entity_id == "never_tracked" + + lineage = prov_mgr.get_lineage("orphan_child") + entity_ids = [e["entity_id"] for e in lineage["lineage_chain"]] + assert entity_ids == ["orphan_child"] + + def test_derived_from_non_string_is_ignored(self): + """A non-string derived_from value (e.g. accidentally passing an int or + list) should be ignored rather than raising or being used as a parent id.""" + prov_mgr = ProvenanceManager() + + entry = prov_mgr.track_entity( + entity_id="entity_bad_derived_from", + source="doc_1", + metadata={"derived_from": 12345}, + ) + + assert entry.parent_entity_id is None + + def test_derived_from_empty_string_is_ignored(self): + """An empty-string derived_from is falsy and should not be treated as a parent link.""" + prov_mgr = ProvenanceManager() + + entry = prov_mgr.track_entity( + entity_id="entity_empty_derived_from", + source="doc_1", + metadata={"derived_from": ""}, + ) + + assert entry.parent_entity_id is None + + def test_derived_from_self_reference_does_not_infinite_loop(self): + """An entity that (incorrectly) declares itself as its own derived_from + parent should not cause get_lineage to hang or infinitely recurse.""" + prov_mgr = ProvenanceManager() + + prov_mgr.track_entity( + entity_id="self_ref", + source="doc_1", + metadata={"derived_from": "self_ref"}, + ) + + lineage = prov_mgr.get_lineage("self_ref") + entity_ids = [e["entity_id"] for e in lineage["lineage_chain"]] + assert entity_ids == ["self_ref"] + + def test_derived_from_multi_hop_chain(self): + """derived_from links should chain transitively: A <- B <- C should + all appear when tracing lineage from C.""" + prov_mgr = ProvenanceManager() + + prov_mgr.track_entity(entity_id="grandparent", source="doc_1") + prov_mgr.track_entity( + entity_id="parent", + source="doc_1", + metadata={"derived_from": "grandparent"}, + ) + prov_mgr.track_entity( + entity_id="child", + source="doc_1", + metadata={"derived_from": "parent"}, + ) + + lineage = prov_mgr.get_lineage("child") + + assert lineage["entity_count"] == 3 + entity_ids = {e["entity_id"] for e in lineage["lineage_chain"]} + assert entity_ids == {"grandparent", "parent", "child"} + + def test_derived_from_without_metadata_dict_does_not_crash(self): + """track_entity called with no metadata at all should behave as before + (no parent link derived), exercising the `metadata and isinstance(...)` guard.""" + prov_mgr = ProvenanceManager() + + entry = prov_mgr.track_entity(entity_id="no_metadata_entity", source="doc_1") + + assert entry.parent_entity_id is None + def test_batch_entity_tracking(self): """Test batch entity tracking.""" prov_mgr = ProvenanceManager() From de0357aec8f2ef9a1e9621acefaa8781c8251dbd Mon Sep 17 00:00:00 2001 From: KaifAhmad1 Date: Wed, 15 Jul 2026 12:27:42 +0530 Subject: [PATCH 2/2] Fix code review findings: metadata precedence and Mapping support - get_lineage() aggregated metadata by iterating trace_lineage()'s BFS order and calling dict.update() on each entry, so ancestor metadata (now reachable via derived_from chains) could overwrite the queried entity's own metadata on conflicting keys. Reverse the iteration so the queried entity (always lineage_entries[0]) is applied last and wins, matching the documented "most recent entry's metadata takes precedence" intent. - track_entity()'s derived_from guard only accepted a concrete dict, silently ignoring other collections.abc.Mapping implementations (e.g. types.MappingProxyType). Switch the isinstance check to Mapping so any mapping-like metadata is honored. Addresses Qodo review findings on PR #741. --- CHANGELOG.md | 4 ++- semantica/provenance/manager.py | 13 +++++++--- tests/provenance/test_manager.py | 42 ++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 438c4f48..a1042e2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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 - `track_entity()` now treats `metadata["derived_from"]` as an explicit parent link (unless `parent_entity_id` was already passed directly), so `InMemoryStorage.trace_lineage()`'s existing BFS over `parent_entity_id` picks it up for free - - Added 7 regression/edge-case tests in `tests/provenance/test_manager.py` covering the happy path, explicit `parent_entity_id` precedence over `derived_from`, precedence over the `source`-as-known-entity-id fallback, a `derived_from` pointing at a never-tracked entity, non-string/empty-string `derived_from` values being ignored, a self-referencing `derived_from` not hanging traversal, and multi-hop `derived_from` chains, closing #735 + - `metadata["derived_from"]` is now recognized on any `collections.abc.Mapping`, not just a concrete `dict`, so e.g. `types.MappingProxyType` metadata still creates the parent link + - `get_lineage()`'s metadata aggregation now applies the queried entity's own metadata last so it wins over ancestor metadata on conflicting keys, matching the documented "most recent entry's metadata takes precedence" behavior — previously `trace_lineage()`'s BFS order caused ancestor metadata (now reachable via `derived_from` chains) to silently overwrite the queried entity's own values + - Added 9 regression/edge-case tests in `tests/provenance/test_manager.py` covering the happy path, explicit `parent_entity_id` precedence over `derived_from`, precedence over the `source`-as-known-entity-id fallback, a `derived_from` pointing at a never-tracked entity, non-string/empty-string `derived_from` values being ignored, a self-referencing `derived_from` not hanging traversal, multi-hop `derived_from` chains, metadata precedence between a queried entity and its ancestors, and non-`dict` `Mapping` metadata, closing #735 - **`InferenceResult.premises` always empty from `forward_chain`/`backward_chain`** (#739) by @Sameer6305 - `_match_rule()` discarded matched facts and returned only instantiated conclusions, so `ExplanationGenerator` always produced empty premises lists regardless of which facts actually satisfied a rule, closing #733 diff --git a/semantica/provenance/manager.py b/semantica/provenance/manager.py index c9f8bb08..94b2f622 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 @@ -118,7 +119,7 @@ class ProvenanceManager: # 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, dict): + 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 @@ -460,10 +461,14 @@ class ProvenanceManager: if not lineage_entries: return {} - # Aggregate metadata from all lineage entries - # Most recent entry's metadata takes precedence + # Aggregate metadata from all lineage entries. + # trace_lineage() is a BFS starting at entity_id, so lineage_entries[0] + # is always the queried entity itself, followed by its ancestors + # (parent, grandparent, ...). Apply ancestors first and the queried + # entity last so its own keys win on conflict, matching the intent + # that the "most recent"/current entity's metadata takes precedence. aggregated_metadata = {} - for entry in lineage_entries: + for entry in reversed(lineage_entries): if entry.metadata: meta = entry.metadata if isinstance(meta, str): diff --git a/tests/provenance/test_manager.py b/tests/provenance/test_manager.py index 0b85f73f..ea0187ae 100644 --- a/tests/provenance/test_manager.py +++ b/tests/provenance/test_manager.py @@ -248,6 +248,48 @@ class TestProvenanceManager: assert entry.parent_entity_id is None + def test_get_lineage_metadata_prefers_queried_entity_over_ancestors(self): + """Aggregated lineage metadata should let the queried entity's own + values win over ancestor values on conflicting keys, matching the + documented "most recent entry's metadata takes precedence" intent.""" + prov_mgr = ProvenanceManager() + + prov_mgr.track_entity( + entity_id="ancestor", + source="doc_1", + metadata={"status": "draft", "shared_only_on_ancestor": True}, + ) + prov_mgr.track_entity( + entity_id="descendant", + source="doc_1", + metadata={"status": "final", "derived_from": "ancestor"}, + ) + + lineage = prov_mgr.get_lineage("descendant") + + assert lineage["metadata"]["status"] == "final" + assert lineage["metadata"]["shared_only_on_ancestor"] is True + + def test_derived_from_accepts_non_dict_mapping(self): + """metadata['derived_from'] should be honored for any Mapping + implementation, not just a concrete dict (e.g. types.MappingProxyType + or a custom collections.abc.Mapping).""" + from types import MappingProxyType + + prov_mgr = ProvenanceManager() + + prov_mgr.track_entity(entity_id="mapping_parent", source="doc_1") + entry = prov_mgr.track_entity( + entity_id="mapping_child", + source="doc_1", + metadata=MappingProxyType({"derived_from": "mapping_parent"}), + ) + + assert entry.parent_entity_id == "mapping_parent" + + lineage = prov_mgr.get_lineage("mapping_child") + assert lineage["entity_count"] == 2 + def test_batch_entity_tracking(self): """Test batch entity tracking.""" prov_mgr = ProvenanceManager()