From 2179d86d92c41eaafd5e36cd43d680760ebc4583 Mon Sep 17 00:00:00 2001 From: Andrew Greene Date: Fri, 27 Mar 2026 19:00:09 -0400 Subject: [PATCH] Define engine-to-integrator error contract (#42) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Establish a typed exception hierarchy and consistent error contract so integrators never see raw ValueError/Neo4j exceptions. VRE rolls back in-memory mutations and re-raises — integrators decide recovery strategy. Key changes: - New core/errors.py with VREError hierarchy (GraphError, PersistenceError, GraphIntegrityError, CyclicRelationshipError, HydrationError, ResolutionError, CandidateValidationError) - Fix silent override: CyclicRelationshipError in _persist_reachability now does rollback + re-raise instead of silently returning SKIPPED - Wrap all PrimitiveRepository methods with GraphError/PersistenceError - Wrap hydration with HydrationError, resolver with ResolutionError - Unknown cardinality defaults to None, firing all policies (safe default) - CyclicRelationshipError re-parented from ValueError to GraphIntegrityError Co-Authored-By: Claude Opus 4.6 (1M context) --- src/vre/__init__.py | 23 +- src/vre/core/__init__.py | 18 +- src/vre/core/errors.py | 45 ++++ src/vre/core/graph.py | 390 ++++++++++++++++------------- src/vre/core/grounding/resolver.py | 7 +- src/vre/core/models.py | 5 - src/vre/core/policy/gate.py | 11 +- src/vre/core/policy/models.py | 15 +- src/vre/learning/engine.py | 28 +-- tests/vre/test_learning.py | 60 ++--- tests/vre/test_vre.py | 6 +- 11 files changed, 369 insertions(+), 239 deletions(-) create mode 100644 src/vre/core/errors.py diff --git a/src/vre/__init__.py b/src/vre/__init__.py index 6b42d03..7441ed3 100644 --- a/src/vre/__init__.py +++ b/src/vre/__init__.py @@ -19,7 +19,17 @@ from vre.core.graph import PrimitiveRepository from vre.core.grounding import ConceptResolver, GroundingEngine, GroundingResult -from vre.core.models import CyclicRelationshipError, DepthLevel, Provenance, ProvenanceSource +from vre.core.errors import ( + CandidateValidationError, + CyclicRelationshipError, + GraphError, + GraphIntegrityError, + HydrationError, + PersistenceError, + ResolutionError, + VREError, +) +from vre.core.models import DepthLevel, Provenance, ProvenanceSource from vre.core.policy import Cardinality, PolicyAction, PolicyCallbackResult, PolicyResult, PolicyViolation from vre.core.policy.callback import PolicyCallContext from vre.core.policy.gate import PolicyGate @@ -33,7 +43,14 @@ __all__ = [ "VRE", + "CandidateValidationError", "CyclicRelationshipError", + "GraphError", + "GraphIntegrityError", + "HydrationError", + "PersistenceError", + "ResolutionError", + "VREError", "PrimitiveRepository", "ConceptResolver", "GroundingEngine", @@ -167,12 +184,12 @@ def check_policy( if grounding.trace is None: return PolicyResult(action=PolicyAction.PASS) - card_enum = Cardinality.SINGLE + card_enum: Cardinality | None = None if cardinality is not None: try: card_enum = Cardinality(cardinality) except ValueError: - pass # unknown string → fall back to SINGLE + card_enum = None # unknown → fire all policies gate = PolicyGate() violations = gate.evaluate(grounding.trace, card_enum, call_context) diff --git a/src/vre/core/__init__.py b/src/vre/core/__init__.py index 3d4acc8..019a428 100644 --- a/src/vre/core/__init__.py +++ b/src/vre/core/__init__.py @@ -1,16 +1,32 @@ # Copyright 2026 Andrew Greene # Licensed under the Apache License, Version 2.0 -from vre.core.models import ( +from vre.core.errors import ( + CandidateValidationError, CyclicRelationshipError, + GraphError, + GraphIntegrityError, + HydrationError, + PersistenceError, + ResolutionError, + VREError, +) +from vre.core.models import ( Provenance, ProvenanceSource, TRANSITIVE_RELATION_TYPES, ) __all__ = [ + "CandidateValidationError", "CyclicRelationshipError", + "GraphError", + "GraphIntegrityError", + "HydrationError", + "PersistenceError", "Provenance", "ProvenanceSource", + "ResolutionError", "TRANSITIVE_RELATION_TYPES", + "VREError", ] diff --git a/src/vre/core/errors.py b/src/vre/core/errors.py new file mode 100644 index 0000000..5f3a94c --- /dev/null +++ b/src/vre/core/errors.py @@ -0,0 +1,45 @@ +# Copyright 2026 Andrew Greene +# Licensed under the Apache License, Version 2.0 + +""" +VRE exception hierarchy. + +All VRE-specific exceptions derive from VREError so integrators can catch +at the desired granularity — from a single error type up to the entire +framework. + +VRE's responsibility is to roll back any in-memory mutations and re-raise +errors with clear, typed exceptions. Integrators decide recovery strategy. +""" + + +class VREError(Exception): + """Base exception for all VRE errors.""" + + +class GraphError(VREError): + """A graph backend operation failed (read or write).""" + + +class PersistenceError(GraphError): + """A write operation against the graph backend failed.""" + + +class GraphIntegrityError(VREError): + """A graph operation would violate structural integrity constraints.""" + + +class CyclicRelationshipError(GraphIntegrityError): + """An edge would create a cycle on transitive relationship types.""" + + +class HydrationError(VREError): + """Failed to reconstruct a domain object from stored data.""" + + +class ResolutionError(VREError): + """Failed to resolve a concept name or identifier.""" + + +class CandidateValidationError(VREError): + """A learning candidate is missing required fields or references invalid data.""" diff --git a/src/vre/core/graph.py b/src/vre/core/graph.py index be0440c..20308b6 100644 --- a/src/vre/core/graph.py +++ b/src/vre/core/graph.py @@ -14,9 +14,15 @@ from uuid import UUID from neo4j import GraphDatabase +from neo4j.exceptions import Neo4jError -from vre.core.models import ( +from vre.core.errors import ( CyclicRelationshipError, + GraphError, + HydrationError, + PersistenceError, +) +from vre.core.models import ( Depth, DepthLevel, EpistemicStep, @@ -77,14 +83,17 @@ def ensure_constraints(self) -> None: """ Create uniqueness constraint on Primitive.id. """ - with self._driver.session(database=self._database) as session: - session.run( - cast( - LiteralString, - "CREATE CONSTRAINT primitive_id_unique IF NOT EXISTS " - "FOR (p:Primitive) REQUIRE p.id IS UNIQUE", + try: + with self._driver.session(database=self._database) as session: + session.run( + cast( + LiteralString, + "CREATE CONSTRAINT primitive_id_unique IF NOT EXISTS " + "FOR (p:Primitive) REQUIRE p.id IS UNIQUE", + ) ) - ) + except Neo4jError as exc: + raise GraphError(f"Failed to ensure constraints: {exc}") from exc @staticmethod def _depths_to_json(depths: list[Depth]) -> str: @@ -110,59 +119,64 @@ def _hydrate_primitive( """ Reconstruct a Primitive from raw Neo4j node data and its relationship records. """ - raw_depths = json.loads(node_data["depths_json"]) - depths_by_level: dict[int, Depth] = {} - for rd in raw_depths: - depth = Depth( - level=DepthLevel(rd["level"]), - properties=rd.get("properties", {}), - provenance=Provenance(**rd["provenance"]) if rd.get("provenance") else None, - ) - depths_by_level[int(depth.level)] = depth - - for rel in relationships: - rel_props = rel.get("rel_props", {}) - source_depth = rel_props.get("source_depth") - target_depth_val = rel_props.get("target_depth") - metadata_json = rel_props.get("metadata_json", "{}") - metadata = json.loads(metadata_json) if metadata_json else {} - - policies = rel_props.get("policies", "[]") - policies_data = json.loads(policies) if policies else [] - policies = [parse_policy(p) for p in policies_data] - - rel_prov_json = rel_props.get("provenance") - rel_prov = None - if rel_prov_json: - rel_prov_data = json.loads(rel_prov_json) if isinstance(rel_prov_json, str) else rel_prov_json - rel_prov = Provenance(**rel_prov_data) - - relatum = Relatum( - relation_type=RelationType(rel["rel_type"]), - target_id=UUID(rel["target_id"]), - target_depth=DepthLevel(target_depth_val), - metadata=metadata, - policies=policies, - provenance=rel_prov, - ) + try: + raw_depths = json.loads(node_data["depths_json"]) + depths_by_level: dict[int, Depth] = {} + for rd in raw_depths: + depth = Depth( + level=DepthLevel(rd["level"]), + properties=rd.get("properties", {}), + provenance=Provenance(**rd["provenance"]) if rd.get("provenance") else None, + ) + depths_by_level[int(depth.level)] = depth + + for rel in relationships: + rel_props = rel.get("rel_props", {}) + source_depth = rel_props.get("source_depth") + target_depth_val = rel_props.get("target_depth") + metadata_json = rel_props.get("metadata_json", "{}") + metadata = json.loads(metadata_json) if metadata_json else {} + + policies = rel_props.get("policies", "[]") + policies_data = json.loads(policies) if policies else [] + policies = [parse_policy(p) for p in policies_data] + + rel_prov_json = rel_props.get("provenance") + rel_prov = None + if rel_prov_json: + rel_prov_data = json.loads(rel_prov_json) if isinstance(rel_prov_json, str) else rel_prov_json + rel_prov = Provenance(**rel_prov_data) + + relatum = Relatum( + relation_type=RelationType(rel["rel_type"]), + target_id=UUID(rel["target_id"]), + target_depth=DepthLevel(target_depth_val), + metadata=metadata, + policies=policies, + provenance=rel_prov, + ) - if source_depth is not None and source_depth in depths_by_level: - depths_by_level[source_depth].relata.append(relatum) + if source_depth is not None and source_depth in depths_by_level: + depths_by_level[source_depth].relata.append(relatum) - sorted_depths = sorted(depths_by_level.values(), key=lambda d: int(d.level)) + sorted_depths = sorted(depths_by_level.values(), key=lambda d: int(d.level)) - node_prov_json = node_data.get("provenance") - node_prov = None - if node_prov_json: - node_prov_data = json.loads(node_prov_json) if isinstance(node_prov_json, str) else node_prov_json - node_prov = Provenance(**node_prov_data) + node_prov_json = node_data.get("provenance") + node_prov = None + if node_prov_json: + node_prov_data = json.loads(node_prov_json) if isinstance(node_prov_json, str) else node_prov_json + node_prov = Provenance(**node_prov_data) - return Primitive( - id=UUID(node_data["id"]), - name=node_data["name"], - depths=sorted_depths, - provenance=node_prov, - ) + return Primitive( + id=UUID(node_data["id"]), + name=node_data["name"], + depths=sorted_depths, + provenance=node_prov, + ) + except (json.JSONDecodeError, KeyError, TypeError, ValueError) as exc: + raise HydrationError( + f"Failed to hydrate primitive '{node_data.get('name', '?')}': {exc}" + ) from exc @staticmethod def _check_transitive_cycles( @@ -284,18 +298,26 @@ def _tx(tx: Any) -> None: provenance=rp["provenance"], ) - with self._driver.session(database=self._database) as session: - session.execute_write(_tx) + try: + with self._driver.session(database=self._database) as session: + session.execute_write(_tx) + except CyclicRelationshipError: + raise + except Neo4jError as exc: + raise PersistenceError(f"Failed to save primitive '{primitive.name}': {exc}") from exc def list_names(self) -> list[str]: """ Return the names of all primitives in the graph, sorted alphabetically. """ - with self._driver.session(database=self._database) as session: - result = session.run( - cast(LiteralString, "MATCH (p:Primitive) RETURN p.name AS name ORDER BY p.name") - ) - return [record["name"] for record in result] + try: + with self._driver.session(database=self._database) as session: + result = session.run( + cast(LiteralString, "MATCH (p:Primitive) RETURN p.name AS name ORDER BY p.name") + ) + return [record["name"] for record in result] + except Neo4jError as exc: + raise GraphError(f"Failed to list primitive names: {exc}") from exc def find_by_id(self, id: UUID) -> Primitive | None: """ @@ -323,33 +345,38 @@ def find_by_id(self, id: UUID) -> Primitive | None: """, ) - with self._driver.session(database=self._database) as session: - record = session.run(cypher, id=str(id)).single() - if record is None or record["id"] is None: - return None - - node_data = { - "id": record["id"], - "name": record["name"], - "depths_json": record["depths_json"], - "provenance": record["provenance"], - } - relationships = [ - { - "rel_type": r["rel_type"], - "target_id": r["target_id"], - "rel_props": { - "source_depth": r["source_depth"], - "target_depth": r["target_depth"], - "metadata_json": r.get("metadata_json") or "{}", - "policies": r.get("policies") or "[]", - "provenance": r.get("provenance"), - }, + try: + with self._driver.session(database=self._database) as session: + record = session.run(cypher, id=str(id)).single() + if record is None or record["id"] is None: + return None + + node_data = { + "id": record["id"], + "name": record["name"], + "depths_json": record["depths_json"], + "provenance": record["provenance"], } - for r in record["rels"] - if r.get("rel_type") is not None - ] - return self._hydrate_primitive(node_data, relationships) + relationships = [ + { + "rel_type": r["rel_type"], + "target_id": r["target_id"], + "rel_props": { + "source_depth": r["source_depth"], + "target_depth": r["target_depth"], + "metadata_json": r.get("metadata_json") or "{}", + "policies": r.get("policies") or "[]", + "provenance": r.get("provenance"), + }, + } + for r in record["rels"] + if r.get("rel_type") is not None + ] + return self._hydrate_primitive(node_data, relationships) + except (GraphError, HydrationError): + raise + except Neo4jError as exc: + raise GraphError(f"Failed to find primitive by id '{id}': {exc}") from exc def find_by_name(self, name: str) -> Primitive | None: """ @@ -378,47 +405,55 @@ def find_by_name(self, name: str) -> Primitive | None: """, ) - with self._driver.session(database=self._database) as session: - record = session.run(cypher, name=name).single() - if record is None or record["id"] is None: - return None - - node_data = { - "id": record["id"], - "name": record["name"], - "depths_json": record["depths_json"], - "provenance": record["provenance"], - } - relationships = [ - { - "rel_type": r["rel_type"], - "target_id": r["target_id"], - "rel_props": { - "source_depth": r["source_depth"], - "target_depth": r["target_depth"], - "metadata_json": r.get("metadata_json") or "{}", - "policies": r.get("policies") or "[]", - "provenance": r.get("provenance"), - }, + try: + with self._driver.session(database=self._database) as session: + record = session.run(cypher, name=name).single() + if record is None or record["id"] is None: + return None + + node_data = { + "id": record["id"], + "name": record["name"], + "depths_json": record["depths_json"], + "provenance": record["provenance"], } - for r in record["rels"] - if r.get("rel_type") is not None - ] - return self._hydrate_primitive(node_data, relationships) + relationships = [ + { + "rel_type": r["rel_type"], + "target_id": r["target_id"], + "rel_props": { + "source_depth": r["source_depth"], + "target_depth": r["target_depth"], + "metadata_json": r.get("metadata_json") or "{}", + "policies": r.get("policies") or "[]", + "provenance": r.get("provenance"), + }, + } + for r in record["rels"] + if r.get("rel_type") is not None + ] + return self._hydrate_primitive(node_data, relationships) + except (GraphError, HydrationError): + raise + except Neo4jError as exc: + raise GraphError(f"Failed to find primitive by name '{name}': {exc}") from exc def delete_primitive(self, id: UUID) -> bool: """ Delete the primitive with the given UUID and all its relationships. Returns True if deleted. """ - with self._driver.session(database=self._database) as session: - result = session.run( - cast( - LiteralString, - "MATCH (p:Primitive {id: $id}) DETACH DELETE p RETURN count(p) AS deleted", - ), - id=str(id), - ).single() - return result is not None and result["deleted"] > 0 + try: + with self._driver.session(database=self._database) as session: + result = session.run( + cast( + LiteralString, + "MATCH (p:Primitive {id: $id}) DETACH DELETE p RETURN count(p) AS deleted", + ), + id=str(id), + ).single() + return result is not None and result["deleted"] > 0 + except Neo4jError as exc: + raise PersistenceError(f"Failed to delete primitive '{id}': {exc}") from exc def resolve_subgraph( self, @@ -466,53 +501,58 @@ def resolve_subgraph( """, ) - with self._driver.session(database=self._database) as session: - record = session.run( - cypher, - names=lowered, - transitive_types=_TRANSITIVE_RELS, - ).single() - - if record is None: - return ResolvedSubgraph(roots=[], nodes=[], edges=[]) - - raw_roots = record["roots"] - raw_nodes = record["nodes"] - raw_edges = record["edges"] - - edges_by_source: dict[str, list[dict[str, Any]]] = {} - for e in raw_edges: - sid = e["source_id"] - edges_by_source.setdefault(sid, []).append({ - "rel_type": e["rel_type"], - "target_id": e["target_id"], - "rel_props": { - "source_depth": e["source_depth"], - "target_depth": e["target_depth"], - "metadata_json": e.get("metadata_json", "{}"), - "policies": e.get("policies", "[]"), - "provenance": e.get("provenance"), - }, - }) - - roots = [ - self._hydrate_primitive(r, edges_by_source.get(r["id"], [])) - for r in raw_roots - ] - nodes = [ - self._hydrate_primitive(n, edges_by_source.get(n["id"], [])) - for n in raw_nodes - ] - - edges = [ - EpistemicStep( - source_id=UUID(e["source_id"]), - target_id=UUID(e["target_id"]), - relation_type=RelationType(e["rel_type"]), - source_depth=DepthLevel(e["source_depth"]), - target_depth=DepthLevel(e["target_depth"]), - ) - for e in raw_edges - ] - - return ResolvedSubgraph(roots=roots, nodes=nodes, edges=edges) + try: + with self._driver.session(database=self._database) as session: + record = session.run( + cypher, + names=lowered, + transitive_types=_TRANSITIVE_RELS, + ).single() + + if record is None: + return ResolvedSubgraph(roots=[], nodes=[], edges=[]) + + raw_roots = record["roots"] + raw_nodes = record["nodes"] + raw_edges = record["edges"] + + edges_by_source: dict[str, list[dict[str, Any]]] = {} + for e in raw_edges: + sid = e["source_id"] + edges_by_source.setdefault(sid, []).append({ + "rel_type": e["rel_type"], + "target_id": e["target_id"], + "rel_props": { + "source_depth": e["source_depth"], + "target_depth": e["target_depth"], + "metadata_json": e.get("metadata_json", "{}"), + "policies": e.get("policies", "[]"), + "provenance": e.get("provenance"), + }, + }) + + roots = [ + self._hydrate_primitive(r, edges_by_source.get(r["id"], [])) + for r in raw_roots + ] + nodes = [ + self._hydrate_primitive(n, edges_by_source.get(n["id"], [])) + for n in raw_nodes + ] + + edges = [ + EpistemicStep( + source_id=UUID(e["source_id"]), + target_id=UUID(e["target_id"]), + relation_type=RelationType(e["rel_type"]), + source_depth=DepthLevel(e["source_depth"]), + target_depth=DepthLevel(e["target_depth"]), + ) + for e in raw_edges + ] + + return ResolvedSubgraph(roots=roots, nodes=nodes, edges=edges) + except (GraphError, HydrationError): + raise + except Neo4jError as exc: + raise GraphError(f"Failed to resolve subgraph for {names}: {exc}") from exc diff --git a/src/vre/core/grounding/resolver.py b/src/vre/core/grounding/resolver.py index 4071c1c..8b8c68c 100644 --- a/src/vre/core/grounding/resolver.py +++ b/src/vre/core/grounding/resolver.py @@ -13,6 +13,7 @@ from functools import lru_cache +from vre.core.errors import ResolutionError from vre.core.graph import PrimitiveRepository @@ -24,10 +25,10 @@ def _nlp(): try: import spacy return spacy.load("en_core_web_sm") - except OSError: - raise RuntimeError( + except OSError as exc: + raise ResolutionError( "spaCy model not found. Run: python -m spacy download en_core_web_sm" - ) + ) from exc def lemmatize(text: str) -> list[str]: diff --git a/src/vre/core/models.py b/src/vre/core/models.py index 7254b4b..178a543 100644 --- a/src/vre/core/models.py +++ b/src/vre/core/models.py @@ -48,11 +48,6 @@ class RelationType(str, Enum): ) -class CyclicRelationshipError(ValueError): - """ - Raised when an edge would create a cycle on transitive relationship types. - """ - class ProvenanceSource(str, Enum): """ diff --git a/src/vre/core/policy/gate.py b/src/vre/core/policy/gate.py index 076802b..60db397 100644 --- a/src/vre/core/policy/gate.py +++ b/src/vre/core/policy/gate.py @@ -17,11 +17,14 @@ class PolicyGate: @staticmethod def _collect_violations( response: EpistemicResponse, - cardinality: Cardinality, + cardinality: Cardinality | None = None, call_context: PolicyCallContext | None = None, ) -> list[PolicyViolation]: """ Walk all APPLIES_TO relata in the trace and collect triggered policy violations. + + When cardinality is None (unknown), all policies fire — we cannot justify + skipping any policy without knowing the cardinality. """ violations: list[PolicyViolation] = [] for primitive in response.result.primitives: @@ -30,7 +33,9 @@ def _collect_violations( if relatum.relation_type != RelationType.APPLIES_TO: continue for policy in relatum.policies: - if policy.trigger_cardinality is not None and policy.trigger_cardinality != cardinality: + if (cardinality is not None + and policy.trigger_cardinality is not None + and policy.trigger_cardinality != cardinality): continue cb = policy.resolve_callback() cb_result: PolicyCallbackResult | None = None @@ -57,7 +62,7 @@ def _collect_violations( def evaluate( self, response: EpistemicResponse, - cardinality: Cardinality = Cardinality.SINGLE, + cardinality: Cardinality | None = None, call_context: PolicyCallContext | None = None, ) -> list[PolicyViolation]: """ diff --git a/src/vre/core/policy/models.py b/src/vre/core/policy/models.py index aa780c4..45d4e30 100644 --- a/src/vre/core/policy/models.py +++ b/src/vre/core/policy/models.py @@ -13,6 +13,8 @@ from pydantic import BaseModel, Field +from vre.core.errors import VREError + if TYPE_CHECKING: from vre.core.policy.callback import PolicyCallback @@ -71,8 +73,17 @@ def resolve_callback(self) -> PolicyCallback | None: if self.callback is None: return None module_path, _, func_name = self.callback.rpartition(".") - module = importlib.import_module(module_path) - return getattr(module, func_name) + if not module_path or not func_name: + raise VREError( + f"Invalid policy callback path '{self.callback}': expected 'module.attr'" + ) + try: + module = importlib.import_module(module_path) + return getattr(module, func_name) + except (ImportError, AttributeError, ValueError) as exc: + raise VREError( + f"Failed to resolve policy callback '{self.callback}': {exc}" + ) from exc def parse_policy(data: dict[str, Any]) -> Policy: diff --git a/src/vre/learning/engine.py b/src/vre/learning/engine.py index 3cde741..c1e9144 100644 --- a/src/vre/learning/engine.py +++ b/src/vre/learning/engine.py @@ -13,8 +13,8 @@ from vre.core.graph import PrimitiveRepository from vre.core.grounding.models import GroundingResult +from vre.core.errors import CandidateValidationError, CyclicRelationshipError from vre.core.models import ( - CyclicRelationshipError, Depth, DepthGap, DepthLevel, @@ -74,7 +74,7 @@ def _resolve_name_to_id(name: str, grounding: GroundingResult) -> UUID: for p in grounding.trace.result.primitives: if p.name.lower() == name.lower(): return p.id - raise ValueError(f"Cannot resolve '{name}' to a primitive ID from the grounding trace") + raise CandidateValidationError(f"Cannot resolve '{name}' to a primitive ID from the grounding trace") class LearningEngine: @@ -98,7 +98,7 @@ def _persist_existence( Persist a new primitive with D0 (auto-generated) and agent-provided D1. """ if candidate.d1 is None: - raise ValueError(f"ExistenceCandidate '{candidate.name}' is missing D1 (identity)") + raise CandidateValidationError(f"ExistenceCandidate '{candidate.name}' is missing D1 (identity)") d0 = Depth( level=DepthLevel.EXISTENCE, @@ -159,11 +159,11 @@ def _persist_depth( Merge new depth levels into an existing primitive and persist. """ if not candidate.new_depths: - raise ValueError(f"DepthCandidate for '{gap.primitive.name}' has no new depths") + raise CandidateValidationError(f"DepthCandidate for '{gap.primitive.name}' has no new depths") existing = self._repo.find_by_id(gap.primitive.id) if existing is None: - raise ValueError(f"Primitive '{gap.primitive.name}' ({gap.primitive.id}) not found") + raise CandidateValidationError(f"Primitive '{gap.primitive.name}' ({gap.primitive.id}) not found") self._merge_depths(existing, candidate.new_depths, provenance) @@ -174,11 +174,11 @@ def _persist_relational( Merge new depth levels into the target primitive and persist. """ if not candidate.new_depths: - raise ValueError(f"RelationalCandidate for '{gap.target.name}' has no new depths") + raise CandidateValidationError(f"RelationalCandidate for '{gap.target.name}' has no new depths") target = self._repo.find_by_id(gap.target.id) if target is None: - raise ValueError(f"Target '{gap.target.name}' ({gap.target.id}) not found") + raise CandidateValidationError(f"Target '{gap.target.name}' ({gap.target.id}) not found") self._merge_depths(target, candidate.new_depths, provenance) @@ -232,12 +232,12 @@ def _persist_reachability( If depth learning is rejected or skipped, edge placement is abandoned. """ if candidate.target_name is None or candidate.relation_type is None: - raise ValueError( + raise CandidateValidationError( f"ReachabilityCandidate for '{gap.primitive.name}' is missing " f"target_name or relation_type" ) if candidate.source_depth_level is None or candidate.target_depth_level is None: - raise ValueError( + raise CandidateValidationError( f"ReachabilityCandidate for '{gap.primitive.name}' is missing " f"source_depth_level or target_depth_level" ) @@ -246,11 +246,11 @@ def _persist_reachability( source = self._repo.find_by_id(gap.primitive.id) if source is None: - raise ValueError(f"Source '{gap.primitive.name}' ({gap.primitive.id}) not found") + raise CandidateValidationError(f"Source '{gap.primitive.name}' ({gap.primitive.id}) not found") target = self._repo.find_by_id(target_id) if target is None: - raise ValueError(f"Target '{candidate.target_name}' ({target_id}) not found") + raise CandidateValidationError(f"Target '{candidate.target_name}' ({target_id}) not found") # Phase 1: learn missing depths on source, then target result = self._learn_missing_depths(source, candidate.source_depth_level, grounding, callback) @@ -280,7 +280,7 @@ def _persist_reachability( self._repo.save_primitive(source) except CyclicRelationshipError: depth_obj.relata.remove(new_relatum) - return CandidateDecision.SKIPPED + raise return CandidateDecision.ACCEPTED @@ -316,9 +316,9 @@ def learn_at( Process the gap at the given index in the grounding result. """ if not grounding.gaps: - raise ValueError("No gaps to learn from") + raise CandidateValidationError("No gaps to learn from") if gap_index < 0 or gap_index >= len(grounding.gaps): - raise ValueError(f"Gap index {gap_index} out of range (0..{len(grounding.gaps) - 1})") + raise CandidateValidationError(f"Gap index {gap_index} out of range (0..{len(grounding.gaps) - 1})") gap = grounding.gaps[gap_index] candidate = TemplateFactory.from_gap(gap) diff --git a/tests/vre/test_learning.py b/tests/vre/test_learning.py index 1a2665d..103726f 100644 --- a/tests/vre/test_learning.py +++ b/tests/vre/test_learning.py @@ -11,8 +11,8 @@ import pytest from vre.core.grounding.models import GroundingResult +from vre.core.errors import CandidateValidationError, CyclicRelationshipError from vre.core.models import ( - CyclicRelationshipError, Depth, DepthGap, DepthLevel, @@ -241,7 +241,7 @@ def test_rejects_missing_d1(self): def callback(candidate, gr, gap): return filled, CandidateDecision.ACCEPTED - with pytest.raises(ValueError, match="missing D1"): + with pytest.raises(CandidateValidationError, match="missing D1"): engine.learn_at(grounding, 0, callback) def test_modified_gets_conversational_provenance(self): @@ -305,7 +305,7 @@ def test_rejects_empty_new_depths(self): def callback(candidate, gr, gap): return filled, CandidateDecision.ACCEPTED - with pytest.raises(ValueError, match="no new depths"): + with pytest.raises(CandidateValidationError, match="no new depths"): engine.learn_at(grounding, 0, callback) @@ -389,7 +389,7 @@ def test_rejects_missing_target_name(self): def callback(candidate, gr, gap): return filled, CandidateDecision.ACCEPTED - with pytest.raises(ValueError, match="missing"): + with pytest.raises(CandidateValidationError, match="missing"): engine.learn_at(grounding, 0, callback) def test_learns_missing_source_depth_before_placing_edge(self): @@ -520,7 +520,7 @@ def test_rejects_unresolvable_target_name(self): def callback(candidate, gr, gap): return filled, CandidateDecision.ACCEPTED - with pytest.raises(ValueError, match="Cannot resolve"): + with pytest.raises(CandidateValidationError, match="Cannot resolve"): engine.learn_at(grounding, 0, callback) @@ -560,7 +560,7 @@ def test_no_gaps_raises(self): engine = LearningEngine(repo) grounding = _grounding_result(grounded=True, gaps=[]) - with pytest.raises(ValueError, match="No gaps"): + with pytest.raises(CandidateValidationError, match="No gaps"): engine.learn_at(grounding, 0, lambda c, g, gap: (None, CandidateDecision.REJECTED)) @@ -591,7 +591,7 @@ def test_out_of_range_raises(self): gap = ExistenceGap(primitive=_primitive("Copy")) grounding = _grounding_result(grounded=False, gaps=[gap]) - with pytest.raises(ValueError, match="out of range"): + with pytest.raises(CandidateValidationError, match="out of range"): engine.learn_at(grounding, 5, lambda c, g, gap: (None, CandidateDecision.REJECTED)) @@ -658,7 +658,7 @@ def test_primitive_not_found_raises(self): def callback(candidate, gr, gap): return filled, CandidateDecision.ACCEPTED - with pytest.raises(ValueError, match="not found"): + with pytest.raises(CandidateValidationError, match="not found"): engine.learn_at(grounding, 0, callback) def test_replaces_existing_depth_level(self): @@ -755,7 +755,7 @@ def test_empty_new_depths_raises(self): def callback(candidate, gr, gap): return filled, CandidateDecision.ACCEPTED - with pytest.raises(ValueError, match="no new depths"): + with pytest.raises(CandidateValidationError, match="no new depths"): engine.learn_at(grounding, 0, callback) def test_target_not_found_raises(self): @@ -776,7 +776,7 @@ def test_target_not_found_raises(self): def callback(candidate, gr, gap): return filled, CandidateDecision.ACCEPTED - with pytest.raises(ValueError, match="not found"): + with pytest.raises(CandidateValidationError, match="not found"): engine.learn_at(grounding, 0, callback) def test_replaces_existing_depth_on_target(self): @@ -901,7 +901,7 @@ def test_missing_depth_levels_raises(self): def callback(candidate, gr, gap): return filled, CandidateDecision.ACCEPTED - with pytest.raises(ValueError, match="missing"): + with pytest.raises(CandidateValidationError, match="missing"): engine.learn_at(grounding, 0, callback) def test_source_not_found_raises(self): @@ -922,7 +922,7 @@ def test_source_not_found_raises(self): def callback(candidate, gr, gap): return filled, CandidateDecision.ACCEPTED - with pytest.raises(ValueError, match="not found"): + with pytest.raises(CandidateValidationError, match="not found"): engine.learn_at(grounding, 0, callback) def test_target_not_found_raises(self): @@ -943,7 +943,7 @@ def test_target_not_found_raises(self): def callback(candidate, gr, gap): return filled, CandidateDecision.ACCEPTED - with pytest.raises(ValueError, match="not found"): + with pytest.raises(CandidateValidationError, match="not found"): engine.learn_at(grounding, 0, callback) def test_learns_missing_target_depth_and_refreshes(self): @@ -1087,10 +1087,10 @@ def _reachability_grounding( class TestCycleDetection: - """Cycle detection via save_primitive → CyclicRelationshipError → SKIPPED.""" + """Cycle detection via save_primitive → CyclicRelationshipError propagated.""" - def test_self_referential_transitive_skipped(self): - """A→A via REQUIRES is a trivial cycle → SKIPPED.""" + def test_self_referential_transitive_raises(self): + """A→A via REQUIRES is a trivial cycle → CyclicRelationshipError.""" a = _primitive("A", depths=[ _depth(DepthLevel.EXISTENCE), _depth(DepthLevel.IDENTITY), @@ -1110,11 +1110,11 @@ def test_self_referential_transitive_skipped(self): def callback(candidate, gr, gap): return filled, CandidateDecision.ACCEPTED - result = engine.learn_at(grounding, 0, callback) - assert result.decision == CandidateDecision.SKIPPED + with pytest.raises(CyclicRelationshipError): + engine.learn_at(grounding, 0, callback) - def test_direct_cycle_skipped(self): - """A→B via REQUIRES exists; B→A via REQUIRES would cycle → SKIPPED.""" + def test_direct_cycle_raises(self): + """A→B via REQUIRES exists; B→A via REQUIRES would cycle → CyclicRelationshipError.""" b = _primitive("B", depths=[ _depth(DepthLevel.EXISTENCE), _depth(DepthLevel.IDENTITY), @@ -1149,11 +1149,11 @@ def test_direct_cycle_skipped(self): def callback(candidate, gr, gap): return filled, CandidateDecision.ACCEPTED - result = engine.learn_at(grounding, 0, callback) - assert result.decision == CandidateDecision.SKIPPED + with pytest.raises(CyclicRelationshipError): + engine.learn_at(grounding, 0, callback) - def test_indirect_cycle_skipped(self): - """A→B→C via DEPENDS_ON exists; C→A would cycle → SKIPPED.""" + def test_indirect_cycle_raises(self): + """A→B→C via DEPENDS_ON exists; C→A would cycle → CyclicRelationshipError.""" c = _primitive("C", depths=[ _depth(DepthLevel.EXISTENCE), _depth(DepthLevel.IDENTITY), @@ -1203,11 +1203,11 @@ def test_indirect_cycle_skipped(self): def callback(candidate, gr, gap): return filled, CandidateDecision.ACCEPTED - result = engine.learn_at(grounding, 0, callback) - assert result.decision == CandidateDecision.SKIPPED + with pytest.raises(CyclicRelationshipError): + engine.learn_at(grounding, 0, callback) - def test_mixed_transitive_types_cycle_skipped(self): - """A→B via REQUIRES exists; B→A via CONSTRAINED_BY would cycle → SKIPPED.""" + def test_mixed_transitive_types_cycle_raises(self): + """A→B via REQUIRES exists; B→A via CONSTRAINED_BY would cycle → CyclicRelationshipError.""" b = _primitive("B", depths=[ _depth(DepthLevel.EXISTENCE), _depth(DepthLevel.IDENTITY), @@ -1242,8 +1242,8 @@ def test_mixed_transitive_types_cycle_skipped(self): def callback(candidate, gr, gap): return filled, CandidateDecision.ACCEPTED - result = engine.learn_at(grounding, 0, callback) - assert result.decision == CandidateDecision.SKIPPED + with pytest.raises(CyclicRelationshipError): + engine.learn_at(grounding, 0, callback) def test_non_transitive_cycle_allowed(self): """A→B via APPLIES_TO exists; B→A via APPLIES_TO is fine → ACCEPTED.""" diff --git a/tests/vre/test_vre.py b/tests/vre/test_vre.py index b8895e7..09d4311 100644 --- a/tests/vre/test_vre.py +++ b/tests/vre/test_vre.py @@ -159,11 +159,11 @@ def test_cardinality_none_triggers_always_on_policy(self): assert vre.check_policy(["write", "file"], cardinality="single").action == PolicyAction.BLOCK assert vre.check_policy(["write", "file"], cardinality="multiple").action == PolicyAction.BLOCK - def test_unknown_cardinality_string_falls_back_to_single(self): - """Unrecognised cardinality string → treated as SINGLE, not an error.""" + def test_unknown_cardinality_string_fires_all_policies(self): + """Unrecognised cardinality string → None → all policies fire (safe default).""" vre = self._setup(Cardinality.MULTIPLE) result = vre.check_policy(["write", "file"], cardinality="bulk_delete_everything") - assert result.action == PolicyAction.PASS # falls back to SINGLE, MULTIPLE policy skipped + assert result.action == PolicyAction.BLOCK # unknown cardinality cannot skip any policy class TestVRECheck: