From f9b1295d140e7003343c9fecf805a67e1734ce19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B1=9F=E4=BF=8A=E6=9D=B0?= Date: Tue, 18 Aug 2026 10:23:26 +0800 Subject: [PATCH 1/6] fix(reasoning): implement RETE alpha/beta matching with Token model (#300) AlphaNode._matches and BetaNode._can_join were placeholder stubs that always returned True, so the Rete network fired every rule for every fact. Add a regex-based unify_condition (reusing Reasoner._match_pattern's approach) that binds ?vars via named groups and enforces repeated-variable and cross-condition binding consistency. Rework propagation around a Token model (facts + bindings) instead of bare facts: AlphaNode emits single-fact tokens, and BetaNode.join merges left/ right tokens, concatenating facts in condition order and returning a merged token only when shared variables agree. This fixes a P1 chained-join defect where rules with three or more conditions lost bindings and accumulated wrong facts at the third join, and a conflicting third condition could spuriously fire. Beta nodes now keep both left/right token memories and join each new token against every token on the opposite side. Also fix an adjacent bug where beta nodes were never wired into their inputs' children, blocking propagation. Adds tests/reasoning/test_rete_engine.py including a TestThreeConditionChain suite (valid match, third-level conflict suppression, insertion-order independence, complete in-order Match.facts, multiple left tokens joining one right fact, parity against Reasoner._match_rule, and reset clearing all token memory). --- .gitignore | Bin 1308 -> 1316 bytes CHANGELOG.md | 8 + semantica/reasoning/rete_engine.py | 241 +++++++++++++++++----- tests/reasoning/test_rete_engine.py | 309 ++++++++++++++++++++++++++++ 4 files changed, 507 insertions(+), 51 deletions(-) create mode 100644 tests/reasoning/test_rete_engine.py diff --git a/.gitignore b/.gitignore index 0e5128d775760155bd99b5882c2f540f27b192de..806da23870e927d31cb62956bc1afda871dfb1b4 100644 GIT binary patch delta 24 fcmbQkwS;TKCq_2CvedjXt<4`8HJDkr7`PY!YvKm{ delta 17 YcmZ3&HHT}%C&tac7}c0rcp11D06O3V>Hq)$ diff --git a/CHANGELOG.md b/CHANGELOG.md index bbd7cfb2..9cfc34e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **RETE engine matched every fact against every rule — `AlphaNode._matches()` and `BetaNode._can_join()` were placeholder stubs that always returned `True`** (closes #300) + - `semantica/reasoning/rete_engine.py` shipped a Rete network whose per-condition alpha test and cross-condition beta join were both `return True` stubs, so `match_patterns()` fired every rule for every fact regardless of predicate, arity, or shared-variable consistency + - New module-level `unify_condition()` reuses the regex-based approach from `Reasoner._match_pattern()`: a condition pattern like `Person(?x)` / `Parent(?x, ?y)` is compiled against a fact's `predicate(arg, ...)` string, `?var` becomes a named capture group, and a variable seen twice within one condition (e.g. `Loves(?x, ?x)`) becomes a backreference, so it only unifies when both positions hold the same value. Returns the bindings dict or `None` + - Reworked propagation to carry partial-match **tokens** instead of bare facts: a new `Token` dataclass bundles the accumulated `facts` with the consistent `bindings`. `AlphaNode` emits a single-fact token per match; `BetaNode.join()` merges a left token with a right token, concatenating their facts in condition order and returning the merged token only when shared variables agree (conflicting values → `None`, no join). Terminal activations carry the full fact list and accumulated bindings through to the emitted match + - This fixes a P1 chained-join defect: rules with three or more conditions (e.g. `Person(?x)`, `Parent(?x, ?y)`, `Located(?y, ?z)`) previously lost bindings and accumulated wrong facts at the third join, and a conflicting third condition could spuriously fire. Beta nodes now keep both `left_tokens` and `right_tokens` memories and join each new token against every token on the opposite side, so deep chains stay binding-consistent and third-level conflicts are correctly suppressed + - Fixed an adjacent network-topology bug surfaced by the above: newly created beta nodes were never appended to their input nodes' `children`, so tokens could not propagate; propagation was reworked to support chained joins and to thread bindings end-to-end + - New `tests/reasoning/test_rete_engine.py`: `unify_condition` unit cases (single/multi variable, literal args, predicate mismatch, repeated-variable equality), alpha match/reject, beta consistent-join vs conflict-reject, end-to-end rules (single-condition fires only the matching fact; multi-condition join fires only on consistent bindings), and a `TestThreeConditionChain` suite (valid three-condition match, third-level conflict suppression, insertion-order independence, `Match.facts` complete and in condition order, multiple left tokens joining one right fact, parity against `Reasoner._match_rule()`, and `reset()` clearing all token memory) + - **`split`/chunking paths bypassed the centralized spaCy model cache, reloading the model on every call** (#1042, closes #998) by @Accute9, reviewed by @Sameer6305 - `semantica/split/methods.py`'s `split_by_sentences()` and `semantica/split/semantic_chunker.py`'s `SemanticChunker.__init__` each called `spacy.load()` directly instead of reusing the process-level cache added in #889/`semantic_extract/methods.py`'s `load_spacy_model()` — every call/construction re-paid the ~120ms model-load cost independently of `NERExtractor`, which already used the cache - Both now route through `load_spacy_model()`, sharing one cached `Language` instance per model name across `split_by_sentences()`, `SemanticChunker`, and `NERExtractor`; a missing model still falls back to regex/paragraph chunking without poisoning the cache for a later successful load diff --git a/semantica/reasoning/rete_engine.py b/semantica/reasoning/rete_engine.py index b6b79359..8a8f715c 100644 --- a/semantica/reasoning/rete_engine.py +++ b/semantica/reasoning/rete_engine.py @@ -33,16 +33,98 @@ Author: Semantica Contributors License: MIT """ -from collections import defaultdict +import re from dataclasses import dataclass, field -from typing import Any, Dict, List, Optional, Set, Tuple +from typing import Any, Dict, List, Optional, Set -from ..utils.exceptions import ProcessingError, ValidationError from ..utils.logging import get_logger from ..utils.progress_tracker import get_progress_tracker from .reasoner import Fact, Rule +def unify_condition( + condition: Any, + fact: Fact, + initial_bindings: Optional[Dict[str, str]] = None, +) -> Optional[Dict[str, str]]: + """Unify a condition pattern against a fact. + + A condition is a pattern string such as ``"Person(?x)"`` or + ``"knows(?x, ?y)"`` where tokens beginning with ``?`` are variables. + The fact is rendered via its ``__str__`` representation + (``predicate(arg1, arg2)``) and matched against the pattern. + + This mirrors ``Reasoner._match_pattern`` but is self-contained so the + RETE engine does not need a live ``Reasoner`` instance. + + Args: + condition: The condition pattern (string). Non-string conditions + are stringified before matching. + fact: The fact to test. + initial_bindings: Bindings already established upstream. Variables + already bound must match the corresponding literal in the fact. + + Returns: + A dict of variable bindings if the fact unifies with the condition, + otherwise ``None``. + """ + bindings = dict(initial_bindings or {}) + pattern = condition if isinstance(condition, str) else str(condition) + fact_str = str(fact) + + # Split on ?var placeholders, escaping only the literal segments so that + # the surrounding parentheses/commas are matched literally. + segments = re.split(r"(\?\w+)", pattern) + seen_vars: Set[str] = set() + p_regex = "" + for seg in segments: + if seg.startswith("?"): + var_name = seg[1:] + if var_name in bindings: + # Already bound — require the exact literal value. + p_regex += re.escape(bindings[var_name]) + elif var_name in seen_vars: + # Same variable used twice — enforce a backreference. + p_regex += f"(?P={var_name})" + else: + p_regex += f"(?P<{var_name}>.+?)" + seen_vars.add(var_name) + else: + p_regex += re.escape(seg) + p_regex = f"^{p_regex}$" + + try: + match = re.match(p_regex, fact_str) + except re.error: + return None + if not match: + return None + + for var, value in match.groupdict().items(): + if var in bindings and bindings[var] != value: + return None # Binding conflict. + bindings[var] = value + return bindings + + +@dataclass +class Token: + """A partial match flowing through the Rete network. + + A token represents an ordered collection of concrete facts that have + been unified so far, together with the consistent variable bindings + accumulated across those facts. + + Alpha nodes emit single-fact tokens. Beta nodes merge a left token and + a right token into a new token whose ``facts`` are the concatenation of + both sides (preserving condition order) and whose ``bindings`` are the + consistent union of both sides. + """ + + facts: List[Fact] = field(default_factory=list) + bindings: Dict[str, str] = field(default_factory=dict) + + @dataclass class Match: """Pattern match.""" @@ -58,7 +140,7 @@ class ReteNode: def __init__(self, node_id: str): self.node_id = node_id - self.children: List["ReteNode"] = [] + self.children: List[ReteNode] = [] class AlphaNode(ReteNode): @@ -67,19 +149,31 @@ class AlphaNode(ReteNode): def __init__(self, node_id: str, condition: Any): super().__init__(node_id) self.condition = condition - self.matches: List[Fact] = [] + # Single-fact tokens produced by unifying each matched fact with + # this node's condition. + self.tokens: List[Token] = [] - def add_fact(self, fact: Fact) -> bool: - """Add fact if it matches condition.""" - if self._matches(fact): - self.matches.append(fact) - return True - return False + def add_fact(self, fact: Fact) -> Optional[Token]: + """Add fact if it matches the condition, returning its token. - def _matches(self, fact: Fact) -> bool: - """Check if fact matches condition.""" - # Simple matching - can be enhanced - return True + Returns the single-fact ``Token`` produced by unification when the + fact matches, otherwise ``None``. + """ + bindings = self._matches(fact) + if bindings is not None: + token = Token(facts=[fact], bindings=dict(bindings)) + self.tokens.append(token) + return token + return None + + def _matches(self, fact: Fact) -> Optional[Dict[str, str]]: + """Check if fact matches the alpha node condition. + + Returns the variable bindings produced by unification if the fact + matches, otherwise ``None``. An empty dict signals a match with no + variables (still distinct from ``None``). + """ + return unify_condition(self.condition, fact) class BetaNode(ReteNode): @@ -89,19 +183,28 @@ class BetaNode(ReteNode): super().__init__(node_id) self.left = left self.right = right - self.matches: List[Tuple[Fact, Fact]] = [] + # Token memories for each side. Incoming tokens are stored here so + # that later-arriving tokens on the opposite side can be joined + # against every token already seen (chained joins). + self.left_tokens: List[Token] = [] + self.right_tokens: List[Token] = [] - def join(self, left_fact: Fact, right_fact: Fact) -> bool: - """Join facts from left and right nodes.""" - if self._can_join(left_fact, right_fact): - self.matches.append((left_fact, right_fact)) - return True - return False + def join(self, left_token: Token, right_token: Token) -> Optional[Token]: + """Join a left token with a right token. - def _can_join(self, left_fact: Fact, right_fact: Fact) -> bool: - """Check if facts can be joined.""" - # Simple join logic - can be enhanced - return True + Returns a new merged ``Token`` (facts concatenated in condition + order, bindings unified) when the two tokens are consistent, + otherwise ``None`` on a binding conflict. + """ + merged = dict(left_token.bindings) + for var, value in right_token.bindings.items(): + if var in merged and merged[var] != value: + return None # Binding conflict — cannot join. + merged[var] = value + return Token( + facts=list(left_token.facts) + list(right_token.facts), + bindings=merged, + ) class TerminalNode(ReteNode): @@ -175,12 +278,16 @@ class ReteEngine: self._add_rule_to_network(rule) self.logger.info( - f"Built Rete network with {len(self.network)} nodes for {len(rules)} rules" + f"Built Rete network with {len(self.network)} nodes " + f"for {len(rules)} rules" ) self.progress_tracker.stop_tracking( tracking_id, status="completed", - message=f"Built Rete network with {len(self.network)} nodes for {len(rules)} rules", + message=( + f"Built Rete network with {len(self.network)} nodes " + f"for {len(rules)} rules" + ), ) except Exception as e: @@ -208,6 +315,10 @@ class ReteEngine: self.node_counter += 1 beta_node = BetaNode(node_id, current, alpha_nodes[i]) self.network[node_id] = beta_node + # Wire the beta node as a child of both its inputs so facts + # propagating from either side reach the join. + current.children.append(beta_node) + alpha_nodes[i].children.append(beta_node) current = beta_node final_node = current else: @@ -238,31 +349,58 @@ class ReteEngine: # Find matching alpha nodes for node_id, node in self.network.items(): if isinstance(node, AlphaNode): - if node.add_fact(fact): - # Propagate to children - self._propagate_from_alpha(node, fact) + token = node.add_fact(fact) + if token is not None: + # Propagate the single-fact token to children. + self._propagate_token(node, token) - def _propagate_from_alpha(self, alpha_node: AlphaNode, fact: Fact) -> None: - """Propagate from alpha node to children.""" - for child in alpha_node.children: + def _propagate_token(self, source: ReteNode, token: Token) -> None: + """Propagate ``token`` (arriving from ``source``) to its children. + + A ``Token`` carries the ordered facts and consistent bindings of a + partial match. Beta children attempt joins and, on success, emit a + new merged token downstream; terminal children turn the token into a + rule activation using the token's complete facts and bindings. + """ + for child in source.children: if isinstance(child, BetaNode): - # Join with matches from left side - for left_fact in alpha_node.matches: - if child.join(left_fact, fact): - # Propagate to children - for grandchild in child.children: - if isinstance(grandchild, TerminalNode): - match = Match( - rule=grandchild.rule, - facts=[left_fact, fact], - confidence=1.0, - ) - grandchild.activate(match) + self._propagate_to_beta(child, source, token) elif isinstance(child, TerminalNode): - # Direct activation - match = Match(rule=child.rule, facts=[fact], confidence=1.0) + match = Match( + rule=child.rule, + facts=list(token.facts), + bindings=dict(token.bindings), + confidence=1.0, + ) child.activate(match) + def _propagate_to_beta( + self, + beta: "BetaNode", + source: ReteNode, + token: Token, + ) -> None: + """Attempt joins at ``beta`` for a token arriving from one side. + + The incoming token is stored in the corresponding side's memory, + then joined against every token already recorded on the opposite + side. Each successful join produces a new merged token that is + propagated further downstream, enabling correct chained joins across + three or more conditions. + """ + if source is beta.left: + beta.left_tokens.append(token) + for right_token in list(beta.right_tokens): + merged = beta.join(token, right_token) + if merged is not None: + self._propagate_token(beta, merged) + elif source is beta.right: + beta.right_tokens.append(token) + for left_token in list(beta.left_tokens): + merged = beta.join(left_token, token) + if merged is not None: + self._propagate_token(beta, merged) + def match_patterns(self, facts: Optional[List[Fact]] = None) -> List[Match]: """ Match patterns using Rete algorithm. @@ -276,7 +414,7 @@ class ReteEngine: tracking_id = self.progress_tracker.start_tracking( module="reasoning", submodule="ReteEngine", - message=f"Matching patterns using Rete algorithm", + message="Matching patterns using Rete algorithm", ) try: @@ -364,9 +502,10 @@ class ReteEngine: self.facts.clear() for node in self.network.values(): if isinstance(node, AlphaNode): - node.matches.clear() + node.tokens.clear() elif isinstance(node, BetaNode): - node.matches.clear() + node.left_tokens.clear() + node.right_tokens.clear() elif isinstance(node, TerminalNode): node.activations.clear() diff --git a/tests/reasoning/test_rete_engine.py b/tests/reasoning/test_rete_engine.py new file mode 100644 index 00000000..f34f4de8 --- /dev/null +++ b/tests/reasoning/test_rete_engine.py @@ -0,0 +1,309 @@ +"""Tests for the RETE engine pattern matching (issue #300). + +These tests verify that ``AlphaNode._matches`` and ``BetaNode._can_join`` no +longer behave like the old always-``True`` stubs, and that the network as a +whole only fires rules whose conditions actually unify with the facts. +""" + +import itertools +import unittest + +from semantica.reasoning.reasoner import Fact, Rule +from semantica.reasoning.rete_engine import ( + AlphaNode, + BetaNode, + ReteEngine, + unify_condition, +) + + +class TestUnifyCondition(unittest.TestCase): + def test_single_variable_binds(self): + fact = Fact("f1", "Person", ["John"]) + bindings = unify_condition("Person(?x)", fact) + self.assertEqual(bindings, {"x": "John"}) + + def test_predicate_mismatch_returns_none(self): + fact = Fact("f1", "Company", ["Google"]) + self.assertIsNone(unify_condition("Person(?x)", fact)) + + def test_two_arguments_bind(self): + fact = Fact("f2", "Parent", ["John", "Mary"]) + bindings = unify_condition("Parent(?x, ?y)", fact) + self.assertEqual(bindings, {"x": "John", "y": "Mary"}) + + def test_literal_argument_must_match(self): + fact = Fact("f3", "Parent", ["John", "Mary"]) + self.assertIsNone(unify_condition("Parent(Bob, ?y)", fact)) + self.assertEqual(unify_condition("Parent(John, ?y)", fact), {"y": "Mary"}) + + def test_repeated_variable_requires_equal_values(self): + loves_self = Fact("f4", "Loves", ["John", "John"]) + loves_other = Fact("f5", "Loves", ["John", "Mary"]) + self.assertEqual(unify_condition("Loves(?x, ?x)", loves_self), {"x": "John"}) + self.assertIsNone(unify_condition("Loves(?x, ?x)", loves_other)) + + +class TestAlphaNode(unittest.TestCase): + def test_matches_stores_bindings(self): + node = AlphaNode("a1", "Person(?x)") + fact = Fact("f1", "Person", ["John"]) + token = node.add_fact(fact) + self.assertIsNotNone(token) + assert token is not None # narrow type for the checker + self.assertEqual(token.facts, [fact]) + self.assertEqual(token.bindings, {"x": "John"}) + self.assertIn(token, node.tokens) + + def test_non_matching_fact_rejected(self): + node = AlphaNode("a1", "Person(?x)") + fact = Fact("f1", "Company", ["Google"]) + self.assertIsNone(node.add_fact(fact)) + self.assertEqual(node.tokens, []) + + +class TestBetaNode(unittest.TestCase): + def test_join_consistent_bindings(self): + left = AlphaNode("a1", "Parent(?x, ?y)") + right = AlphaNode("a2", "Person(?x)") + beta = BetaNode("b1", left, right) + + parent = Fact("f1", "Parent", ["John", "Mary"]) + person = Fact("f2", "Person", ["John"]) + left_token = left.add_fact(parent) + right_token = right.add_fact(person) + assert left_token is not None and right_token is not None + + merged = beta.join(left_token, right_token) + self.assertIsNotNone(merged) + assert merged is not None # narrow type for the checker + self.assertEqual(merged.bindings, {"x": "John", "y": "Mary"}) + # Facts are concatenated left-then-right in condition order. + self.assertEqual(merged.facts, [parent, person]) + + def test_join_conflicting_bindings_rejected(self): + left = AlphaNode("a1", "Parent(?x, ?y)") + right = AlphaNode("a2", "Person(?x)") + beta = BetaNode("b1", left, right) + + parent = Fact("f1", "Parent", ["John", "Mary"]) + # ?x conflicts: John vs Alice + person = Fact("f2", "Person", ["Alice"]) + left_token = left.add_fact(parent) + right_token = right.add_fact(person) + assert left_token is not None and right_token is not None + + self.assertIsNone(beta.join(left_token, right_token)) + + +class TestReteEngineEndToEnd(unittest.TestCase): + def test_only_matching_rule_fires(self): + engine = ReteEngine() + rule = Rule( + rule_id="r1", + name="person rule", + conditions=["Person(?x)"], + conclusion="Mortal(?x)", + ) + engine.build_network([rule]) + + engine.add_fact(Fact("f1", "Person", ["John"])) + engine.add_fact(Fact("f2", "Company", ["Google"])) # should NOT fire + + matches = engine.match_patterns() + self.assertEqual(len(matches), 1) + self.assertEqual(matches[0].bindings, {"x": "John"}) + + def test_multi_condition_join(self): + engine = ReteEngine() + rule = Rule( + rule_id="r1", + name="child rule", + conditions=["Person(?x)", "Parent(?x, ?y)"], + conclusion="Child(?y, ?x)", + ) + engine.build_network([rule]) + + engine.add_fact(Fact("f1", "Person", ["John"])) + engine.add_fact(Fact("f2", "Parent", ["John", "Mary"])) + # Unrelated parent whose ?x does not match any Person -> no activation. + engine.add_fact(Fact("f3", "Parent", ["Bob", "Sue"])) + + matches = engine.match_patterns() + self.assertEqual(len(matches), 1) + self.assertEqual(matches[0].bindings, {"x": "John", "y": "Mary"}) + + def test_no_activation_when_join_inconsistent(self): + engine = ReteEngine() + rule = Rule( + rule_id="r1", + name="child rule", + conditions=["Person(?x)", "Parent(?x, ?y)"], + conclusion="Child(?y, ?x)", + ) + engine.build_network([rule]) + + engine.add_fact(Fact("f1", "Person", ["John"])) + engine.add_fact(Fact("f2", "Parent", ["Alice", "Mary"])) # ?x mismatch + + matches = engine.match_patterns() + self.assertEqual(matches, []) + + +class TestThreeConditionChain(unittest.TestCase): + """Chained beta joins across three or more conditions (issue #300). + + These exercise the Token model: a token must accumulate the ordered + facts and the consistent bindings of every condition, so that deep + chains neither drop bindings nor duplicate facts, and a conflict on the + third condition correctly suppresses activation. + """ + + def _three_condition_rule(self): + return Rule( + rule_id="r1", + name="location chain", + conditions=[ + "Person(?x)", + "Parent(?x, ?y)", + "Located(?y, ?z)", + ], + conclusion="LivesNear(?x, ?z)", + ) + + def test_three_condition_valid_match(self): + engine = ReteEngine() + engine.build_network([self._three_condition_rule()]) + + engine.add_fact(Fact("f1", "Person", ["John"])) + engine.add_fact(Fact("f2", "Parent", ["John", "Mary"])) + engine.add_fact(Fact("f3", "Located", ["Mary", "Paris"])) + + matches = engine.match_patterns() + self.assertEqual(len(matches), 1) + self.assertEqual( + matches[0].bindings, + {"x": "John", "y": "Mary", "z": "Paris"}, + ) + + def test_three_condition_third_level_conflict(self): + engine = ReteEngine() + engine.build_network([self._three_condition_rule()]) + + engine.add_fact(Fact("f1", "Person", ["John"])) + engine.add_fact(Fact("f2", "Parent", ["John", "Mary"])) + # ?y is bound to Mary, so a Located fact about Bob must not join. + engine.add_fact(Fact("f3", "Located", ["Bob", "Paris"])) + + matches = engine.match_patterns() + self.assertEqual(matches, []) + + def test_fact_insertion_order_independent(self): + # Whatever order facts arrive, the same single match must result. + base_facts = [ + Fact("f1", "Person", ["John"]), + Fact("f2", "Parent", ["John", "Mary"]), + Fact("f3", "Located", ["Mary", "Paris"]), + ] + expected = {"x": "John", "y": "Mary", "z": "Paris"} + + for order in itertools.permutations(base_facts): + engine = ReteEngine() + engine.build_network([self._three_condition_rule()]) + for fact in order: + engine.add_fact(fact) + matches = engine.match_patterns() + self.assertEqual(len(matches), 1, f"order={order}") + self.assertEqual(matches[0].bindings, expected) + + def test_match_facts_complete_in_condition_order(self): + engine = ReteEngine() + engine.build_network([self._three_condition_rule()]) + + person = Fact("f1", "Person", ["John"]) + parent = Fact("f2", "Parent", ["John", "Mary"]) + located = Fact("f3", "Located", ["Mary", "Paris"]) + engine.add_fact(person) + engine.add_fact(parent) + engine.add_fact(located) + + matches = engine.match_patterns() + self.assertEqual(len(matches), 1) + # All three facts preserved, in condition order, no duplicates. + self.assertEqual(matches[0].facts, [person, parent, located]) + + def test_multiple_left_tokens_join_one_right_fact(self): + # Two Person/Parent chains sharing the same Located(?y, ?z) fact. + engine = ReteEngine() + engine.build_network([self._three_condition_rule()]) + + engine.add_fact(Fact("f1", "Person", ["John"])) + engine.add_fact(Fact("f2", "Parent", ["John", "Mary"])) + engine.add_fact(Fact("f3", "Person", ["Alice"])) + engine.add_fact(Fact("f4", "Parent", ["Alice", "Mary"])) + # One right fact should join with both accumulated left tokens. + engine.add_fact(Fact("f5", "Located", ["Mary", "Paris"])) + + matches = engine.match_patterns() + self.assertEqual(len(matches), 2) + result = {m.bindings["x"]: m.bindings["z"] for m in matches} + self.assertEqual(result, {"John": "Paris", "Alice": "Paris"}) + + def test_matches_reasoner_match_rule(self): + from semantica.reasoning.reasoner import Reasoner + + rule = self._three_condition_rule() + facts = [ + Fact("f1", "Person", ["John"]), + Fact("f2", "Parent", ["John", "Mary"]), + Fact("f3", "Located", ["Mary", "Paris"]), + ] + + # Reasoner works over stringified facts and returns + # (conclusion, matched_facts) tuples from self.facts. + reasoner = Reasoner() + for fact in facts: + reasoner.add_fact(str(fact)) + reasoner_matches = reasoner._match_rule(rule) + + engine = ReteEngine() + engine.build_network([rule]) + for fact in facts: + engine.add_fact(fact) + rete_matches = engine.match_patterns() + + # Both engines must agree on the number of activations. + self.assertEqual(len(rete_matches), len(reasoner_matches)) + self.assertEqual(len(rete_matches), 1) + self.assertEqual( + rete_matches[0].bindings, + {"x": "John", "y": "Mary", "z": "Paris"}, + ) + # The RETE match must carry the instantiated conclusion facts too. + conclusion, _ = reasoner_matches[0] + self.assertEqual(conclusion, "LivesNear(John, Paris)") + + def test_reset_clears_all_token_memory(self): + engine = ReteEngine() + engine.build_network([self._three_condition_rule()]) + + engine.add_fact(Fact("f1", "Person", ["John"])) + engine.add_fact(Fact("f2", "Parent", ["John", "Mary"])) + engine.add_fact(Fact("f3", "Located", ["Mary", "Paris"])) + self.assertEqual(len(engine.match_patterns()), 1) + + engine.reset() + + # No stale facts, tokens or activations remain anywhere. + self.assertEqual(engine.facts, []) + for node in engine.network.values(): + if isinstance(node, AlphaNode): + self.assertEqual(node.tokens, []) + elif isinstance(node, BetaNode): + self.assertEqual(node.left_tokens, []) + self.assertEqual(node.right_tokens, []) + self.assertEqual(engine.match_patterns(), []) + + +if __name__ == "__main__": + unittest.main() From a94cec3b3698f7b1bf8f85ebb9e033a432a1faaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B1=9F=E4=BF=8A=E6=9D=B0?= Date: Tue, 18 Aug 2026 12:49:33 +0800 Subject: [PATCH 2/6] fix(reasoning): log unify_condition regex errors for observability Previously unify_condition() silently caught re.error and returned None with no log context, unlike Reasoner._match_pattern() which logs the pattern/regex/fact on failure. This made malformed conditions hard to diagnose in the RETE engine. - Add a module-level logger ("semantica.rete_engine") for the standalone unify_condition() helper. - On re.error, log a WARNING including the condition pattern, compiled regex, and fact string before returning None. - Also catch unexpected exceptions (noqa BLE001) with the same context, mirroring Reasoner._match_pattern behaviour. - Add tests asserting both error paths log a warning and return None. Refs #300 --- semantica/reasoning/rete_engine.py | 22 ++++++++++++++++++++- tests/reasoning/test_rete_engine.py | 30 +++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/semantica/reasoning/rete_engine.py b/semantica/reasoning/rete_engine.py index 8a8f715c..5eca9fc9 100644 --- a/semantica/reasoning/rete_engine.py +++ b/semantica/reasoning/rete_engine.py @@ -41,6 +41,8 @@ from ..utils.logging import get_logger from ..utils.progress_tracker import get_progress_tracker from .reasoner import Fact, Rule +logger = get_logger("rete_engine") + def unify_condition( condition: Any, @@ -95,7 +97,25 @@ def unify_condition( try: match = re.match(p_regex, fact_str) - except re.error: + except re.error as e: + logger.warning( + "unify_condition failed to compile/match condition " + "%r (regex: %r) against fact %r: %s", + pattern, + p_regex, + fact_str, + e, + ) + return None + except Exception as e: # noqa: BLE001 - mirror Reasoner._match_pattern + logger.warning( + "unify_condition unexpected error matching condition " + "%r (regex: %r) against fact %r: %s", + pattern, + p_regex, + fact_str, + e, + ) return None if not match: return None diff --git a/tests/reasoning/test_rete_engine.py b/tests/reasoning/test_rete_engine.py index f34f4de8..f2faec28 100644 --- a/tests/reasoning/test_rete_engine.py +++ b/tests/reasoning/test_rete_engine.py @@ -6,8 +6,11 @@ whole only fires rules whose conditions actually unify with the facts. """ import itertools +import re import unittest +from unittest import mock +from semantica.reasoning import rete_engine from semantica.reasoning.reasoner import Fact, Rule from semantica.reasoning.rete_engine import ( AlphaNode, @@ -43,6 +46,33 @@ class TestUnifyCondition(unittest.TestCase): self.assertEqual(unify_condition("Loves(?x, ?x)", loves_self), {"x": "John"}) self.assertIsNone(unify_condition("Loves(?x, ?x)", loves_other)) + def test_regex_error_logs_warning_and_returns_none(self): + """A regex compilation error is logged with context and yields None.""" + fact = Fact("f6", "Person", ["John"]) + with mock.patch.object( + rete_engine.re, + "match", + side_effect=re.error("bad pattern"), + ), self.assertLogs("semantica.rete_engine", level="WARNING") as captured: + result = unify_condition("Person(?x)", fact) + self.assertIsNone(result) + joined = "\n".join(captured.output) + self.assertIn("Person(?x)", joined) + self.assertIn("Person(John)", joined) + self.assertIn("bad pattern", joined) + + def test_unexpected_error_logs_warning_and_returns_none(self): + """An unexpected error is also logged and swallowed as None.""" + fact = Fact("f7", "Person", ["John"]) + with mock.patch.object( + rete_engine.re, + "match", + side_effect=RuntimeError("boom"), + ), self.assertLogs("semantica.rete_engine", level="WARNING") as captured: + result = unify_condition("Person(?x)", fact) + self.assertIsNone(result) + self.assertIn("boom", "\n".join(captured.output)) + class TestAlphaNode(unittest.TestCase): def test_matches_stores_bindings(self): From b57079451552ba531ef22e13a5dcdf5fa62fa24a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B1=9F=E4=BF=8A=E6=9D=B0?= Date: Tue, 18 Aug 2026 13:36:53 +0800 Subject: [PATCH 3/6] perf(reasoning): precompile alpha node condition regex unify_condition() rebuilt a regex (re.split + concat + re.match) for every fact tested against every alpha node. Since RETE evaluates many facts across many alpha nodes, this repeated construction added significant overhead. - Extract regex construction into _build_condition_regex() (reused by unify_condition and AlphaNode). - AlphaNode.__init__ now compiles its condition once (no initial bindings at alpha time) into self._compiled and reuses it per fact. - On compile failure, log a WARNING and treat the node as non-matching, consistent with the earlier observability fix. - Add tests for the compiled path and the compile-failure fallback. Refs #300 --- semantica/reasoning/rete_engine.py | 103 ++++++++++++++++++++++------ tests/reasoning/test_rete_engine.py | 27 ++++++++ 2 files changed, 109 insertions(+), 21 deletions(-) diff --git a/semantica/reasoning/rete_engine.py b/semantica/reasoning/rete_engine.py index 5eca9fc9..59e0514f 100644 --- a/semantica/reasoning/rete_engine.py +++ b/semantica/reasoning/rete_engine.py @@ -44,6 +44,47 @@ from .reasoner import Fact, Rule logger = get_logger("rete_engine") +def _build_condition_regex( + pattern: str, + initial_bindings: Optional[Dict[str, str]] = None, +) -> str: + """Build an anchored regex string for a condition pattern. + + Splits the pattern on ``?var`` placeholders, escaping the literal + segments so surrounding parentheses/commas match literally. Variables + become named groups (or backreferences when repeated); variables already + present in ``initial_bindings`` are inlined as their literal value. + + Args: + pattern: The condition pattern string (e.g. ``"Person(?x)"``). + initial_bindings: Bindings already established upstream. Variables + already bound are matched as literals rather than captured. + + Returns: + An anchored regex string (``^...$``) suitable for ``re.compile`` / + ``re.match``. + """ + bindings = initial_bindings or {} + segments = re.split(r"(\?\w+)", pattern) + seen_vars: Set[str] = set() + p_regex = "" + for seg in segments: + if seg.startswith("?"): + var_name = seg[1:] + if var_name in bindings: + # Already bound — require the exact literal value. + p_regex += re.escape(bindings[var_name]) + elif var_name in seen_vars: + # Same variable used twice — enforce a backreference. + p_regex += f"(?P={var_name})" + else: + p_regex += f"(?P<{var_name}>.+?)" + seen_vars.add(var_name) + else: + p_regex += re.escape(seg) + return f"^{p_regex}$" + + def unify_condition( condition: Any, fact: Fact, @@ -74,26 +115,9 @@ def unify_condition( pattern = condition if isinstance(condition, str) else str(condition) fact_str = str(fact) - # Split on ?var placeholders, escaping only the literal segments so that - # the surrounding parentheses/commas are matched literally. - segments = re.split(r"(\?\w+)", pattern) - seen_vars: Set[str] = set() - p_regex = "" - for seg in segments: - if seg.startswith("?"): - var_name = seg[1:] - if var_name in bindings: - # Already bound — require the exact literal value. - p_regex += re.escape(bindings[var_name]) - elif var_name in seen_vars: - # Same variable used twice — enforce a backreference. - p_regex += f"(?P={var_name})" - else: - p_regex += f"(?P<{var_name}>.+?)" - seen_vars.add(var_name) - else: - p_regex += re.escape(seg) - p_regex = f"^{p_regex}$" + # Build the anchored regex once (variables already bound are inlined as + # literals). See ``_build_condition_regex`` for the segment handling. + p_regex = _build_condition_regex(pattern, bindings) try: match = re.match(p_regex, fact_str) @@ -172,6 +196,22 @@ class AlphaNode(ReteNode): # Single-fact tokens produced by unifying each matched fact with # this node's condition. self.tokens: List[Token] = [] + # Pre-compile the condition regex once. Alpha nodes never have + # initial bindings, so the pattern is stable for the node's lifetime + # and every incoming fact reuses this compiled matcher instead of + # rebuilding it (avoids repeated regex construction overhead). + pattern = condition if isinstance(condition, str) else str(condition) + self._compiled: Optional[re.Pattern] = None + try: + self._compiled = re.compile(_build_condition_regex(pattern)) + except re.error as e: + logger.warning( + "AlphaNode %r failed to compile condition %r: %s; " + "node will never match", + node_id, + pattern, + e, + ) def add_fact(self, fact: Fact) -> Optional[Token]: """Add fact if it matches the condition, returning its token. @@ -189,11 +229,32 @@ class AlphaNode(ReteNode): def _matches(self, fact: Fact) -> Optional[Dict[str, str]]: """Check if fact matches the alpha node condition. + Uses the pre-compiled regex built in ``__init__`` for performance, + since RETE evaluates many facts against every alpha node. + Returns the variable bindings produced by unification if the fact matches, otherwise ``None``. An empty dict signals a match with no variables (still distinct from ``None``). """ - return unify_condition(self.condition, fact) + if self._compiled is None: + # Compilation failed at build time; treat as non-matching. + return None + fact_str = str(fact) + try: + match = self._compiled.match(fact_str) + except Exception as e: # noqa: BLE001 - mirror unify_condition + logger.warning( + "AlphaNode %r unexpected error matching condition " + "%r against fact %r: %s", + self.node_id, + self.condition, + fact_str, + e, + ) + return None + if not match: + return None + return match.groupdict() class BetaNode(ReteNode): diff --git a/tests/reasoning/test_rete_engine.py b/tests/reasoning/test_rete_engine.py index f2faec28..d6ef29a4 100644 --- a/tests/reasoning/test_rete_engine.py +++ b/tests/reasoning/test_rete_engine.py @@ -91,6 +91,33 @@ class TestAlphaNode(unittest.TestCase): self.assertIsNone(node.add_fact(fact)) self.assertEqual(node.tokens, []) + def test_uses_precompiled_regex(self): + """AlphaNode compiles its condition once and reuses it per fact.""" + node = AlphaNode("a1", "Person(?x)") + self.assertIsNotNone(node._compiled) + # Matching goes through the compiled matcher, not unify_condition. + with mock.patch.object(rete_engine, "unify_condition") as unify: + fact = Fact("f1", "Person", ["John"]) + token = node.add_fact(fact) + unify.assert_not_called() + self.assertIsNotNone(token) + assert token is not None + self.assertEqual(token.bindings, {"x": "John"}) + + def test_bad_condition_never_matches_and_logs(self): + """A condition that fails to compile logs a warning and never fires.""" + with mock.patch.object( + rete_engine, + "_build_condition_regex", + return_value="(unbalanced", + ), self.assertLogs("semantica.rete_engine", level="WARNING") as captured: + node = AlphaNode("bad", "Person(?x)") + self.assertIsNone(node._compiled) + self.assertIn("failed to compile", "\n".join(captured.output)) + fact = Fact("f1", "Person", ["John"]) + self.assertIsNone(node.add_fact(fact)) + self.assertEqual(node.tokens, []) + class TestBetaNode(unittest.TestCase): def test_join_consistent_bindings(self): From ab86127e4e3f2e9aa33ea39dd28eb2dc749abc40 Mon Sep 17 00:00:00 2001 From: dex0shubham Date: Fri, 21 Aug 2026 16:03:22 +0100 Subject: [PATCH 4/6] test: guard fastapi-dependent modules so collection succeeds without the explorer extra Closes #1167 --- tests/explorer/test_explorer_api.py | 19 +++++++++---------- tests/explorer/test_explorer_auth.py | 17 ++++++++--------- tests/explorer/test_ontology_dns_pinning.py | 7 ++++++- tests/explorer/test_ontology_ssrf.py | 7 ++++++- tests/explorer/test_ontology_subissue3.py | 19 +++++++++---------- .../test_provenance_manager_wiring.py | 17 +++++++++++------ tests/explorer/test_provenance_route.py | 9 ++++++++- tests/explorer/test_sparql_route.py | 19 +++++++++---------- tests/explorer/test_vocabulary.py | 17 ++++++++++++----- tests/test_security_regression.py | 7 ++++++- 10 files changed, 84 insertions(+), 54 deletions(-) diff --git a/tests/explorer/test_explorer_api.py b/tests/explorer/test_explorer_api.py index 18d7fcaf..dd8fa18b 100644 --- a/tests/explorer/test_explorer_api.py +++ b/tests/explorer/test_explorer_api.py @@ -9,16 +9,15 @@ import networkx as nx import pytest from semantica.context.context_graph import ContextGraph -from semantica.explorer.app import create_app -from semantica.explorer.session import GraphSession +# fastapi ships in the optional `explorer` extra, not in `dev`, so this module +# must skip rather than fail collection when it is absent. The guard has to sit +# above the import below, which pulls fastapi in transitively. +pytest.importorskip("fastapi") -try: - from starlette.testclient import TestClient -except ImportError: - pytest.skip( - "starlette TestClient is required for explorer tests. Install semantica[explorer].", - allow_module_level=True, - ) +from semantica.explorer.app import create_app # noqa: E402 +from semantica.explorer.session import GraphSession # noqa: E402 + +from starlette.testclient import TestClient # noqa: E402 @@ -1104,7 +1103,7 @@ class TestBidirectionalPathRoute: # _classify_distance unit tests — issue #472 # --------------------------------------------------------------------------- -from semantica.utils.helpers import classify_path_distance +from semantica.utils.helpers import classify_path_distance # noqa: E402 class _FakeSimilarity: diff --git a/tests/explorer/test_explorer_auth.py b/tests/explorer/test_explorer_auth.py index e29461d2..aedcd938 100644 --- a/tests/explorer/test_explorer_auth.py +++ b/tests/explorer/test_explorer_auth.py @@ -13,16 +13,15 @@ browsers can't set custom headers on a WebSocket handshake. import pytest from semantica.context.context_graph import ContextGraph -from semantica.explorer.app import create_app -from semantica.explorer.session import GraphSession +# fastapi ships in the optional `explorer` extra, not in `dev`, so this module +# must skip rather than fail collection when it is absent. The guard has to sit +# above the import below, which pulls fastapi in transitively. +pytest.importorskip("fastapi") -try: - from starlette.testclient import TestClient -except ImportError: - pytest.skip( - "starlette TestClient is required for explorer tests. Install semantica[explorer].", - allow_module_level=True, - ) +from semantica.explorer.app import create_app # noqa: E402 +from semantica.explorer.session import GraphSession # noqa: E402 + +from starlette.testclient import TestClient # noqa: E402 def _build_sample_graph() -> ContextGraph: diff --git a/tests/explorer/test_ontology_dns_pinning.py b/tests/explorer/test_ontology_dns_pinning.py index a68ff192..4801a22f 100644 --- a/tests/explorer/test_ontology_dns_pinning.py +++ b/tests/explorer/test_ontology_dns_pinning.py @@ -27,7 +27,12 @@ import threading import pytest -from semantica.explorer.routes import ontology as ontology_mod +# fastapi ships in the optional `explorer` extra, not in `dev`, so this module +# must skip rather than fail collection when it is absent. The guard has to sit +# above the import below, which pulls fastapi in transitively. +pytest.importorskip("fastapi") + +from semantica.explorer.routes import ontology as ontology_mod # noqa: E402 def _start_local_server(): diff --git a/tests/explorer/test_ontology_ssrf.py b/tests/explorer/test_ontology_ssrf.py index 8613275c..c0013d9b 100644 --- a/tests/explorer/test_ontology_ssrf.py +++ b/tests/explorer/test_ontology_ssrf.py @@ -21,7 +21,12 @@ from unittest.mock import MagicMock, patch import pytest -from semantica.explorer.routes import ontology as ontology_mod +# fastapi ships in the optional `explorer` extra, not in `dev`, so this module +# must skip rather than fail collection when it is absent. The guard has to sit +# above the import below, which pulls fastapi in transitively. +pytest.importorskip("fastapi") + +from semantica.explorer.routes import ontology as ontology_mod # noqa: E402 def _fake_getaddrinfo(host, *args, **kwargs): diff --git a/tests/explorer/test_ontology_subissue3.py b/tests/explorer/test_ontology_subissue3.py index 56800c50..66a660b0 100644 --- a/tests/explorer/test_ontology_subissue3.py +++ b/tests/explorer/test_ontology_subissue3.py @@ -6,17 +6,16 @@ from urllib.parse import quote import pytest from semantica.context.context_graph import ContextGraph -from semantica.explorer.app import create_app -from semantica.explorer.routes.ontology import OntologyEntry -from semantica.explorer.session import GraphSession +# fastapi ships in the optional `explorer` extra, not in `dev`, so this module +# must skip rather than fail collection when it is absent. The guard has to sit +# above the import below, which pulls fastapi in transitively. +pytest.importorskip("fastapi") -try: - from starlette.testclient import TestClient -except ImportError: - pytest.skip( - "starlette TestClient is required for explorer tests. Install semantica[explorer].", - allow_module_level=True, - ) +from semantica.explorer.app import create_app # noqa: E402 +from semantica.explorer.routes.ontology import OntologyEntry # noqa: E402 +from semantica.explorer.session import GraphSession # noqa: E402 + +from starlette.testclient import TestClient # noqa: E402 def _build_ontology_graph() -> ContextGraph: diff --git a/tests/explorer/test_provenance_manager_wiring.py b/tests/explorer/test_provenance_manager_wiring.py index 282a48cc..86d9ad12 100644 --- a/tests/explorer/test_provenance_manager_wiring.py +++ b/tests/explorer/test_provenance_manager_wiring.py @@ -5,13 +5,18 @@ Tests for ProvenanceManager wiring into Explorer routes and application startup. from unittest.mock import patch import pytest -from starlette.testclient import TestClient +# fastapi ships in the optional `explorer` extra, not in `dev`, so this module +# must skip rather than fail collection when it is absent. The guard has to sit +# above the starlette/explorer imports below, which need that extra. +pytest.importorskip("fastapi") -from semantica.context.context_graph import ContextGraph -from semantica.explorer.app import create_app -from semantica.explorer.session import GraphSession -from semantica.provenance import ProvenanceManager -from semantica.provenance.storage import SQLiteStorage +from starlette.testclient import TestClient # noqa: E402 + +from semantica.context.context_graph import ContextGraph # noqa: E402 +from semantica.explorer.app import create_app # noqa: E402 +from semantica.explorer.session import GraphSession # noqa: E402 +from semantica.provenance import ProvenanceManager # noqa: E402 +from semantica.provenance.storage import SQLiteStorage # noqa: E402 @pytest.fixture diff --git a/tests/explorer/test_provenance_route.py b/tests/explorer/test_provenance_route.py index aa2b9d72..caea248a 100644 --- a/tests/explorer/test_provenance_route.py +++ b/tests/explorer/test_provenance_route.py @@ -2,7 +2,14 @@ from types import SimpleNamespace -from semantica.explorer.routes.provenance import ( +import pytest + +# fastapi ships in the optional `explorer` extra, not in `dev`, so this module +# must skip rather than fail collection when it is absent. The guard has to sit +# above the import below, which pulls fastapi in transitively. +pytest.importorskip("fastapi") + +from semantica.explorer.routes.provenance import ( # noqa: E402 _add_chain_edges, _build_provenance, _render_markdown, diff --git a/tests/explorer/test_sparql_route.py b/tests/explorer/test_sparql_route.py index 1f619aa7..b2335f80 100644 --- a/tests/explorer/test_sparql_route.py +++ b/tests/explorer/test_sparql_route.py @@ -14,18 +14,17 @@ from unittest.mock import patch import pytest from semantica.context.context_graph import ContextGraph -from semantica.explorer.app import create_app -from semantica.explorer.session import GraphSession +# fastapi ships in the optional `explorer` extra, not in `dev`, so this module +# must skip rather than fail collection when it is absent. The guard has to sit +# above the import below, which pulls fastapi in transitively. +pytest.importorskip("fastapi") -try: - from starlette.testclient import TestClient -except ImportError: - pytest.skip( - "starlette TestClient is required for explorer tests. Install semantica[explorer].", - allow_module_level=True, - ) +from semantica.explorer.app import create_app # noqa: E402 +from semantica.explorer.session import GraphSession # noqa: E402 -import semantica.explorer.routes.sparql as sparql_mod +from starlette.testclient import TestClient # noqa: E402 + +import semantica.explorer.routes.sparql as sparql_mod # noqa: E402 def _build_sample_graph() -> ContextGraph: diff --git a/tests/explorer/test_vocabulary.py b/tests/explorer/test_vocabulary.py index 3de0f3e9..b3e67a20 100644 --- a/tests/explorer/test_vocabulary.py +++ b/tests/explorer/test_vocabulary.py @@ -2,12 +2,19 @@ from unittest.mock import MagicMock -from fastapi import FastAPI -from fastapi.testclient import TestClient +import pytest -from semantica.explorer.dependencies import get_session -from semantica.explorer.routes.vocabulary import router -from semantica.utils.skos import validate_skos_hierarchy +# fastapi ships in the optional `explorer` extra, not in `dev`, so this module +# must skip rather than fail collection when it is absent. The guard has to sit +# above the import below, which pulls fastapi in transitively. +pytest.importorskip("fastapi") + +from fastapi import FastAPI # noqa: E402 +from fastapi.testclient import TestClient # noqa: E402 + +from semantica.explorer.dependencies import get_session # noqa: E402 +from semantica.explorer.routes.vocabulary import router # noqa: E402 +from semantica.utils.skos import validate_skos_hierarchy # noqa: E402 app = FastAPI() app.include_router(router) diff --git a/tests/test_security_regression.py b/tests/test_security_regression.py index 56ad46de..cc4ae542 100644 --- a/tests/test_security_regression.py +++ b/tests/test_security_regression.py @@ -24,7 +24,12 @@ import pytest # rdf:/rdfs: namespaces) and neither the code nor this test caught it, # since both had the same bug. Importing the real function makes that class # of drift impossible. -from semantica.explorer.routes.sparql import _is_read_only_query +# fastapi ships in the optional `explorer` extra, not in `dev`, so this module +# must skip rather than fail collection when it is absent. The guard has to sit +# above the import below, which pulls fastapi in transitively. +pytest.importorskip("fastapi") + +from semantica.explorer.routes.sparql import _is_read_only_query # noqa: E402 class TestSparqlReadOnlyValidation: From c8a591e89e48b0cb723914782b81fdc1aaf289cf Mon Sep 17 00:00:00 2001 From: dex0shubham Date: Fri, 28 Aug 2026 20:00:10 +0100 Subject: [PATCH 5/6] test: scope the security-regression guard to the SPARQL class, guard the new decision-route test Addresses review feedback on #1232. --- .../explorer/test_decision_route_timestamp.py | 19 ++++++++++++------- tests/test_security_regression.py | 18 ++++++++++++------ 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/tests/explorer/test_decision_route_timestamp.py b/tests/explorer/test_decision_route_timestamp.py index 4b568619..3baffaac 100644 --- a/tests/explorer/test_decision_route_timestamp.py +++ b/tests/explorer/test_decision_route_timestamp.py @@ -24,14 +24,19 @@ import math from datetime import datetime import pytest -from fastapi.testclient import TestClient -from pydantic import ValidationError +# fastapi ships in the optional `explorer` extra, not in `dev`, so this module +# must skip rather than fail collection when it is absent. The guard has to sit +# above the imports below, which need that extra. +pytest.importorskip("fastapi") -from semantica.context.context_graph import ContextGraph -from semantica.explorer.app import create_app -from semantica.explorer.routes.decisions import _node_to_decision -from semantica.explorer.schemas import DecisionResponse -from semantica.explorer.session import GraphSession +from fastapi.testclient import TestClient # noqa: E402 +from pydantic import ValidationError # noqa: E402 + +from semantica.context.context_graph import ContextGraph # noqa: E402 +from semantica.explorer.app import create_app # noqa: E402 +from semantica.explorer.routes.decisions import _node_to_decision # noqa: E402 +from semantica.explorer.schemas import DecisionResponse # noqa: E402 +from semantica.explorer.session import GraphSession # noqa: E402 # --------------------------------------------------------------------------- diff --git a/tests/test_security_regression.py b/tests/test_security_regression.py index cc4ae542..f133e6ba 100644 --- a/tests/test_security_regression.py +++ b/tests/test_security_regression.py @@ -24,14 +24,20 @@ import pytest # rdf:/rdfs: namespaces) and neither the code nor this test caught it, # since both had the same bug. Importing the real function makes that class # of drift impossible. -# fastapi ships in the optional `explorer` extra, not in `dev`, so this module -# must skip rather than fail collection when it is absent. The guard has to sit -# above the import below, which pulls fastapi in transitively. -pytest.importorskip("fastapi") - -from semantica.explorer.routes.sparql import _is_read_only_query # noqa: E402 +# fastapi ships in the optional `explorer` extra, not in `dev`, so this import +# fails on a plain dev install. Only the SPARQL class below needs it; the Cypher, +# XXE, vector-serialization and SSRF classes in this module are independent, so +# the skip is scoped to the one class rather than the whole file. +try: + from semantica.explorer.routes.sparql import _is_read_only_query +except ImportError: # pragma: no cover - depends on the installed extras + _is_read_only_query = None +@pytest.mark.skipif( + _is_read_only_query is None, + reason="requires semantica[explorer] (fastapi)", +) class TestSparqlReadOnlyValidation: """Regression tests for SPARQL injection prevention.""" From 47446ebdde7d5da4c2b3804e74e801962479a760 Mon Sep 17 00:00:00 2001 From: dex0shubham Date: Sat, 29 Aug 2026 13:31:00 +0100 Subject: [PATCH 6/6] test(explorer): guard the deterministic-rendering e2e module on fastapi This module landed after the branch was opened and imports semantica.explorer.app at module scope, so it reproduced the same collection error on a clean [dev] install. --- .../test_explorer_deterministic_rendering_e2e.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/explorer/test_explorer_deterministic_rendering_e2e.py b/tests/explorer/test_explorer_deterministic_rendering_e2e.py index b4b99c3c..83bb2837 100644 --- a/tests/explorer/test_explorer_deterministic_rendering_e2e.py +++ b/tests/explorer/test_explorer_deterministic_rendering_e2e.py @@ -15,9 +15,14 @@ from pathlib import Path import pytest -from semantica.context.context_graph import ContextGraph -from semantica.explorer.app import create_app -from semantica.explorer.session import GraphSession +# fastapi ships in the optional `explorer` extra, not in `dev`, so this module +# must skip rather than fail collection when it is absent. The guard has to sit +# above the explorer imports below, which pull fastapi in transitively. +pytest.importorskip("fastapi") + +from semantica.context.context_graph import ContextGraph # noqa: E402 +from semantica.explorer.app import create_app # noqa: E402 +from semantica.explorer.session import GraphSession # noqa: E402 try: from starlette.testclient import TestClient