From ad9ea48d26f519661d07c4d442b1ecfbb34adc33 Mon Sep 17 00:00:00 2001 From: KaifAhmad1 Date: Tue, 17 Mar 2026 17:01:26 +0530 Subject: [PATCH] fix: resolve review issues in DatalogReasoner - Remove forced progress_tracker.enabled=True (was mutating global singleton) - Wrap derive_all() fixpoint loop in try/finally so stop_tracking is always called - Add _derived flag to cache fixpoint result; query() no longer re-runs derive_all() on every call - Reset _derived to False in add_fact(), add_rule(), and clear() - Warn (instead of silently drop) when add_fact() receives an unrecognised dict format - Fix syntax error on line 9 of test file (stray dashes caused SyntaxError, broke CI) - Add missing TestContextGraphIntegration tests: test_edge_becomes_fact and test_derive_after_load - All 18 tests pass Co-Authored-By: KaifAhmad1 Co-Authored-By: Claude Sonnet 4.6 --- semantica/reasoning/datalog_reasoner.py | 88 ++++++++++++++---------- tests/reasoning/test_datalog_reasoner.py | 36 ++++++++-- 2 files changed, 82 insertions(+), 42 deletions(-) diff --git a/semantica/reasoning/datalog_reasoner.py b/semantica/reasoning/datalog_reasoner.py index de4d2101..fcb042e9 100644 --- a/semantica/reasoning/datalog_reasoner.py +++ b/semantica/reasoning/datalog_reasoner.py @@ -48,14 +48,13 @@ class DatalogReasoner: self.config.update(kwargs) self.progress_tracker = get_progress_tracker() - if not self.progress_tracker.enabled: - self.progress_tracker.enabled = True self._fact_index: Dict[str, Set[DatalogFact]] = defaultdict(set) self._all_facts: Set[DatalogFact] = set() - + self._rules: List[DatalogRule] = [] - + self._derived: bool = False + self._delta_old: Set[DatalogFact] = set() self._delta_new: Set[DatalogFact] = set() @@ -64,6 +63,7 @@ class DatalogReasoner: self._fact_index.clear() self._all_facts.clear() self._rules.clear() + self._derived = False self._delta_old.clear() self._delta_new.clear() @@ -109,14 +109,20 @@ class DatalogReasoner: if arg[0].isupper(): raise ValueError(f"Facts must be constants only. Found variable '{arg}' in {fact}") + if parsed_fact is None and isinstance(fact, dict): + self.logger.warning(f"Unrecognised dict fact format, skipping: {fact}") + return + if parsed_fact and parsed_fact not in self._all_facts: self._all_facts.add(parsed_fact) self._fact_index[parsed_fact.predicate].add(parsed_fact) - + self._derived = False + def add_rule(self, rule_str: str) -> None: """ Add a Datalog rule using Horn clause syntax.""" rule = self._parse_rule_string(rule_str) self._rules.append(rule) + self._derived = False # Parsing helpers @@ -238,46 +244,52 @@ class DatalogReasoner: Executes bottom-up semi-naive evaluation until fixpoint is reached. Returns a list of all derived facts as strings. """ + if self._derived: + return [f"{f.predicate}({', '.join(f.args)})" for f in self._all_facts] + tracking_id = self.progress_tracker.start_tracking( module="reasoning", submodule="DatalogReasoner", message="Starting semi-naive fixpoint evaluation" ) - + iteration = 0 newly_derived_count = 0 - - self._delta_new = self._all_facts.copy() - - while self._delta_new: - iteration += 1 - - # Shift deltas - self._delta_old = self._delta_new - self._delta_new = set() - - delta_index = defaultdict(set) - for f in self._delta_old: - delta_index[f.predicate].add(f) - - for rule in self._rules: - new_facts = self._apply_rule(rule, delta_index) - - for fact in new_facts: - if fact not in self._all_facts: - self._delta_new.add(fact) - self._all_facts.add(fact) - self._fact_index[fact.predicate].add(fact) - newly_derived_count += 1 - - self.logger.debug(f"Datalog Iteration {iteration}: derived {len(self._delta_new)} new facts") - self.progress_tracker.stop_tracking( - tracking_id, - status="completed", - message=f"Fixpoint reached in {iteration} iterations. {newly_derived_count} new facts derived." - ) - + try: + self._delta_new = self._all_facts.copy() + + while self._delta_new: + iteration += 1 + + # Shift deltas + self._delta_old = self._delta_new + self._delta_new = set() + + delta_index = defaultdict(set) + for f in self._delta_old: + delta_index[f.predicate].add(f) + + for rule in self._rules: + new_facts = self._apply_rule(rule, delta_index) + + for fact in new_facts: + if fact not in self._all_facts: + self._delta_new.add(fact) + self._all_facts.add(fact) + self._fact_index[fact.predicate].add(fact) + newly_derived_count += 1 + + self.logger.debug(f"Datalog Iteration {iteration}: derived {len(self._delta_new)} new facts") + + self._derived = True + finally: + self.progress_tracker.stop_tracking( + tracking_id, + status="completed", + message=f"Fixpoint reached in {iteration} iterations. {newly_derived_count} new facts derived." + ) + return [f"{f.predicate}({', '.join(f.args)})" for f in self._all_facts] def _apply_rule( @@ -335,7 +347,7 @@ class DatalogReasoner: Syntax: "ancestor(tom, ?Y)" or "ancestor(tom, ?y)" Returns: [{"Y": "bob"}] or [{"y": "bob"}] """ - if self._rules: + if self._rules and not self._derived: self.derive_all() match = re.match(r'^\s*([a-zA-Z0-9_]+)\s*\(\s*([^)]+)\s*\)\s*\.?\s*$', pattern.strip()) diff --git a/tests/reasoning/test_datalog_reasoner.py b/tests/reasoning/test_datalog_reasoner.py index 4d7a82bc..421d38bc 100644 --- a/tests/reasoning/test_datalog_reasoner.py +++ b/tests/reasoning/test_datalog_reasoner.py @@ -6,7 +6,6 @@ import pytest from typing import List, Dict, Any from semantica.reasoning.datalog_reasoner import DatalogReasoner, DatalogFact ----------------------------------------------------------------- @pytest.fixture def reasoner(): @@ -135,13 +134,42 @@ class TestContextGraphIntegration: nodes=[{"id": "microsoft", "type": "company"}], edges=[{"source": "microsoft", "target": "openai", "type": "invested_in"}] ) - + added = reasoner.load_from_graph(graph) - assert added == 2 - + assert added == 2 + assert DatalogFact("company", ("microsoft",)) in reasoner._all_facts assert DatalogFact("invested_in", ("microsoft", "openai")) in reasoner._all_facts + def test_edge_becomes_fact(self, reasoner): + graph = MockContextGraph( + nodes=[], + edges=[ + {"source": "alice", "target": "bob", "type": "manages"}, + {"source": "bob", "target": "carol", "type": "manages"}, + ] + ) + reasoner.load_from_graph(graph) + + assert DatalogFact("manages", ("alice", "bob")) in reasoner._all_facts + assert DatalogFact("manages", ("bob", "carol")) in reasoner._all_facts + + def test_derive_after_load(self, reasoner): + graph = MockContextGraph( + nodes=[], + edges=[ + {"source": "alice", "target": "bob", "type": "manages"}, + {"source": "bob", "target": "carol", "type": "manages"}, + ] + ) + reasoner.load_from_graph(graph) + + reasoner.add_rule("transitive_manages(X, Y) :- manages(X, Y).") + reasoner.add_rule("transitive_manages(X, Y) :- manages(X, Z), transitive_manages(Z, Y).") + + derived = reasoner.derive_all() + assert "transitive_manages(alice, carol)" in derived + class TestEdgeCases: def test_empty_program(self, reasoner):