Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Prevent circular connections between nodes #321

Merged
merged 10 commits into from
Feb 13, 2025

Conversation

gentamura
Copy link
Contributor

@gentamura gentamura commented Jan 22, 2025

Summary

Implements validation to prevent circular dependencies in node connections, including self-references and connection loops. This ensures stable graph execution by preventing infinite loops and unpredictable behavior.

Related Issue

Closes #297

Changes

  • Added validateConnection function to check for circular dependencies
  • Implemented self-reference prevention in node connections
  • Added cycle detection using DFS algorithm
  • Enhanced error handling with specific GraphError types
  • Added user feedback when circular connections are attempted
  • Implemented comprehensive test cases for connection validation

Testing

  • Added test cases for self-reference prevention
  • Added test cases for circular dependency detection
  • Added test cases for valid connection scenarios
  • Verified error messages and user feedback

Other Information

The implementation uses an efficient DFS algorithm for cycle detection, ensuring good performance even with larger node graphs. The validation is performed before connection creation, providing immediate feedback to users when invalid connections are attempted.

- Add validateConnection function to check node connections
- Implement validation for self-references and circular dependencies
- Throw errors for invalid connections in deriveFlows
- Add comprehensive test cases for connection validation

This commit adds validation to prevent circular dependencies and
self-references in node connections, while allowing other operations
like node deletion to proceed normally. The validation includes:
- Self-reference detection
- Circular dependency detection using DFS
- Node existence validation
- Node type validation
Copy link

vercel bot commented Jan 22, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
giselle ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 13, 2025 0:44am

Add try-catch-finally blocks to ensure proper cleanup of persist timer
even when applyActions throws an error. This maintains consistency
between graph state updates and persistence scheduling.

- Wrap graph state updates in try block
- Add error logging and re-throw in catch block
- Move timer cleanup and scheduling to finally block
Add GraphError class to provide structured error handling with user-friendly
messages and system-level details. This improves error reporting when
validating graph connections.

- Add GraphError class with error codes and dual-message system
- Update validateConnection to use GraphError for all error cases
- Update deriveFlows to handle GraphError properly
- Add error handling in properties panel when adding connections
- Improve error messages to be more user-friendly
Update test assertions to check GraphError properties including:
- error instance type
- error code
- user-friendly message
- system message

This aligns the tests with the new structured error handling
implemented in the previous commit.
…ions

The validateConnection function now checks for self-reference in both:
- The new connection being added
- Any existing connections in the graph
- Update self-reference check in validateConnection
- Add test case for detecting self-reference in existing connections
Remove NODE_NOT_FOUND and TYPE_MISMATCH error checks from validateConnection as they are redundant:
- NODE_NOT_FOUND is prevented by UI constraints and active connection filtering
- TYPE_MISMATCH cannot occur as connection types are derived from actual nodes
@gentamura
Copy link
Contributor Author

@shige @toyamarinyon review, please 🙏

Copy link
Member

@shige shige left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🟢

@gentamura
Copy link
Contributor Author

Thank you for reviewers 🚀

@gentamura gentamura merged commit d47c20c into main Feb 13, 2025
9 checks passed
@gentamura gentamura deleted the feature/prevent-circular-connections branch February 13, 2025 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prevent Circular Node Connections
3 participants