Add cycle detection on transitive relationship types#40
Open
anormang1992 wants to merge 1 commit intomainfrom
Open
Add cycle detection on transitive relationship types#40anormang1992 wants to merge 1 commit intomainfrom
anormang1992 wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds cycle prevention for transitive relationship edges so resolve_subgraph’s unbounded traversal can’t be corrupted by cycles, and updates the learning loop to skip candidates that would introduce such cycles.
Changes:
- Introduces
TRANSITIVE_RELATION_TYPESandCyclicRelationshipErrorincore.models, exporting them via package__init__modules. - Adds transitive-cycle detection to Neo4j
PrimitiveRepository.save_primitive()and updates the learning engine to treat cycle attempts asSKIPPED. - Adds/updates learning tests and enhances the test
StubRepositorywith BFS-based cycle checks.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/vre/test_learning.py | Adds cycle-detection tests and implements matching cycle checks in the in-memory test repository. |
| src/vre/learning/engine.py | Catches CyclicRelationshipError during reachability persistence and returns SKIPPED; removes dead target refetch. |
| src/vre/core/models.py | Centralizes transitive relation types and defines a dedicated cycle error type. |
| src/vre/core/graph.py | Adds Neo4j-side cycle detection in save_primitive() using a reachability query; derives transitive type list from the centralized set. |
| src/vre/core/init.py | Re-exports CyclicRelationshipError and TRANSITIVE_RELATION_TYPES. |
| src/vre/init.py | Exposes CyclicRelationshipError at the top-level public API and updates parameter docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Prevent cyclical edges on transitive relation types (REQUIRES, DEPENDS_ON, CONSTRAINED_BY) which would cause unbounded traversal in resolve_subgraph. Cycle check runs inside the save_primitive transaction after edge deletion but before creation — Neo4j rolls back atomically if a cycle is detected. The learning engine catches CyclicRelationshipError and returns SKIPPED so the loop continues to the next gap. Also removes dead target refetch in _persist_reachability and centralizes the transitive type set as TRANSITIVE_RELATION_TYPES in models.py. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
c25a2e7 to
42965d6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
save_primitive()that prevents edges on transitive relation types (REQUIRES,DEPENDS_ON,CONSTRAINED_BY) from forming cycles, which would cause unbounded traversal inresolve_subgraphCyclicRelationshipErrorand returnsSKIPPEDso the learning loop continues to the next gapTRANSITIVE_RELATION_TYPESinmodels.py, replacing the private_TRANSITIVE_RELSstring list ingraph.py_persist_reachabilityCloses #33
Test plan
🤖 Generated with Claude Code