Define engine-to-integrator error contract

[anormang1992]

Summary

Closes #42.

• New core/errors.py — typed exception hierarchy rooted at VREError. Integrators can catch at desired granularity (individual error type → VREError catch-all).
• Fix silent override — CyclicRelationshipError in _persist_reachability now does rollback + re-raise instead of silently converting to SKIPPED. The engine cleans up its in-memory mutation; the integrator decides recovery strategy.
• Wrap repository boundary — all PrimitiveRepository methods catch Neo4jError and raise GraphError (reads) or PersistenceError (writes). Integrators never see raw Neo4j driver exceptions.
• Wrap hydration — corrupted JSON in the database raises HydrationError instead of raw json.JSONDecodeError/KeyError.
• Safe cardinality default — unknown cardinality strings default to None, which fires all policies. If we can't determine the cardinality, we can't justify skipping any policy.
• CyclicRelationshipError re-parented from ValueError to GraphIntegrityError(VREError). Breaking change — clean break, no backward-compat shim.

Error hierarchy

VREError
├── GraphError                    (any graph backend failure)
│   └── PersistenceError          (write-specific: save, delete)
├── GraphIntegrityError
│   └── CyclicRelationshipError   (re-parented from ValueError)
├── HydrationError                (corrupt data on read)
├── ResolutionError               (concept/name resolution)
└── CandidateValidationError      (bad candidate fields in learning)

Test plan

• All 194 existing tests pass (updated for new exception types)
• Cycle detection tests expect CyclicRelationshipError to propagate (not SKIPPED)
• Cardinality test updated: unknown string → all policies fire → BLOCK
• CandidateValidationError replaces ValueError in all learning engine validation
• Coverage at 74.53% (above 70% threshold)

Implementation plan

Issue #42: Engine-to-Integrator Error Contract

Context

The LearningEngine silently overrides integrator decisions when post-decision failures occur (engine.py:279-283). More broadly, VRE lacks a consistent error contract — many errors propagate as raw ValueError or untyped exceptions, making it hard for integrators to handle them programmatically.

Principle: VRE's responsibility is to roll back any in-memory mutations and re-raise errors with clear, typed exceptions. Integrators decide recovery strategy. VRE does not make recovery decisions on behalf of integrators.

Guard decorator: Unhandled exceptions propagating out of vre_guard is acceptable — the integrator wraps the decorated function call in their own error handling.

---

Part 1: Error Hierarchy — new src/vre/core/errors.py

Dedicated module for the exception hierarchy. Keeps models.py focused on domain models.

VREError (base for all VRE exceptions)
├── GraphError                          # any graph backend failure (reads + writes)
│   └── PersistenceError                # write-specific failures (save, delete)
├── GraphIntegrityError
│   └── CyclicRelationshipError         # re-parented from ValueError
├── HydrationError                      # JSON/data corruption on read
├── ResolutionError                     # concept/name resolution failures
└── CandidateValidationError            # invalid candidate fields in learning

• CyclicRelationshipError moves from core/models.py to core/errors.py. Clean break — no re-export from models.py.
• Re-parented from ValueError to GraphIntegrityError(VREError). Breaking change — acceptable.
• All existing imports update to from vre.core.errors import CyclicRelationshipError (or from vre import CyclicRelationshipError).

---

Part 2: Fix the Silent Override

File: src/vre/learning/engine.py:279-283

# Before — silent override:
try:
    self._repo.save_primitive(source)
except CyclicRelationshipError:
    depth_obj.relata.remove(new_relatum)
    return CandidateDecision.SKIPPED

# After — rollback + re-raise:
try:
    self._repo.save_primitive(source)
except CyclicRelationshipError:
    depth_obj.relata.remove(new_relatum)  # engine's cleanup responsibility
    raise                                  # integrator's decision

Exception propagates through learn_at → learn_all → integrator. The with callback: context manager ensures __exit__ fires for cleanup.

---

Part 3: Wrap Critical Sections

3a. Repository boundary — Neo4j (src/vre/core/graph.py)

Wrap all public methods so integrators never see raw Neo4j exceptions. Use GraphError for reads, PersistenceError for writes:

Method	Exception
save_primitive()	PersistenceError (let CyclicRelationshipError + provenance ValueError pass through)
delete_primitive()	PersistenceError
find_by_id()	GraphError
find_by_name()	GraphError
list_names()	GraphError
resolve_subgraph()	GraphError
ensure_constraints()	GraphError

3b. Hydration (src/vre/core/graph.py)

Wrap _hydrate_primitive with HydrationError for JSON/data corruption.

3c. Concept resolution (src/vre/core/grounding/resolver.py)

Replace RuntimeError with ResolutionError for missing spaCy model.

3d. Learning engine validation (src/vre/learning/engine.py)

Replace raw ValueError with CandidateValidationError in all _persist_* methods, _resolve_name_to_id, and learn_at.

3e. Policy callback resolution (src/vre/core/policy/models.py)

Wrap resolve_callback() with VREError for ImportError/AttributeError.

3f. Cardinality — safe default on bad input

Default to None on unknown cardinality. Update the gate to fire all policies when cardinality is unknown — if we can't determine cardinality, we can't justify skipping any policy.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Defines a typed, integrator-facing error contract across the VRE stack, ensuring engine/repository/grounding layers re-raise consistent VREError subclasses (instead of raw/untyped exceptions) and avoid silently overriding integrator decisions.

Changes:

• Introduces vre.core.errors exception hierarchy and re-exports error types from vre.core and top-level vre.
• Updates learning engine validation + cycle handling to raise typed exceptions and propagate CyclicRelationshipError (with in-memory rollback).
• Wraps Neo4j repository operations and hydration to prevent raw driver/JSON exceptions from leaking to integrators; updates policy cardinality handling to “safe default” behavior.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/vre/core/errors.py	Adds new VRE exception hierarchy.
src/vre/core/__init__.py	Re-exports new error types from vre.core.
src/vre/__init__.py	Re-exports error types from top-level package; adjusts cardinality parsing to safe-default behavior.
src/vre/core/graph.py	Wraps Neo4jError at repository boundary and wraps hydration failures with HydrationError.
src/vre/core/grounding/resolver.py	Uses ResolutionError when spaCy model is missing.
src/vre/learning/engine.py	Replaces ValueError with CandidateValidationError; propagates CyclicRelationshipError after rollback.
src/vre/core/policy/models.py	Wraps policy callback dotted-path resolution failures as VREError.
src/vre/core/policy/gate.py	Allows cardinality=None to mean “unknown → do not skip any policies.”
src/vre/core/models.py	Removes CyclicRelationshipError from models (moved to errors module).
tests/vre/test_learning.py	Updates tests for typed learning validation errors and cycle propagation behavior.
tests/vre/test_vre.py	Updates tests for unknown-cardinality safe-default policy behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

resolve_callback() only wraps ImportError/AttributeError, but importlib.import_module(module_path) can raise ValueError when self.callback isn’t a valid dotted path (e.g. missing module part → empty module name). This would leak a raw ValueError to integrators. Validate that module_path/func_name are non-empty and/or include ValueError in the wrapped exceptions so all resolution failures surface as VREError.

Suggested change

	try:
	module = importlib.import_module(module_path)
	return getattr(module, func_name)
	except (ImportError, AttributeError) as exc:
	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:

[Copilot]

The multi-line condition indentation here is inconsistent and may trigger Ruff/pycodestyle continuation-indentation violations (and it’s harder to read). Consider either keeping the condition on one line (it appears to fit within the 120 char limit) or re-indenting the continuation lines to align consistently under the opening parenthesis.

Suggested change

	if (cardinality is not None
	and 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):

[Copilot]

This re-raises as ResolutionError but drops the original OSError context. Using exception chaining (raise ... from exc) would preserve the underlying error details for debugging while still enforcing the typed error contract.

[Copilot]

CyclicRelationshipError is currently a GraphError, but it represents a graph integrity/constraint violation (cycle detection) rather than a backend/IO failure. This also diverges from the PR description’s hierarchy (mentions GraphIntegrityError with CyclicRelationshipError under it). Consider introducing GraphIntegrityError(VREError) and subclassing CyclicRelationshipError from that, so integrators can distinguish integrity errors from backend failures when catching exceptions.

Suggested change

	class PersistenceError(GraphError):
	"""A write operation against the graph backend failed."""
	class CyclicRelationshipError(GraphError):
	class GraphIntegrityError(VREError):
	"""The graph violates an integrity or constraint rule."""
	class PersistenceError(GraphError):
	"""A write operation against the graph backend failed."""
	class CyclicRelationshipError(GraphIntegrityError):