Fix topological sort violating hook dependencies by using Kahn's algorithm with priority tie-breaking

[Copilot]

The topologicalSort function computed a valid dependency-respecting order via DFS, but then re-sorted the result by priority, breaking @DependsOnHook constraints.

Changes

• Refactored to use Kahn's algorithm: Replaced custom DFS implementation with Kahn's algorithm pattern consistent with existing kahn-topological-sort.kt in the codebase
• Priority queue for tie-breaking: Uses PriorityQueue instead of ArrayDeque to naturally handle priority-based ordering when multiple hooks have no dependencies
• Maintained topological guarantees: Dependencies always execute before dependents, with priority acting as a tie-breaker for independent hooks

Implementation

The refactored implementation:

1. Builds a dependency graph mapping each hook to its dependents
2. Calculates incoming edges for each hook using fastutil's Object2IntMap
3. Uses a PriorityQueue ordered by hook priority (lower value = higher priority)
4. Applies Kahn's algorithm to process hooks with zero incoming edges
5. Detects circular dependencies when not all hooks can be processed

Example

Before this fix, with hooks:

• Hook A (priority=10, no dependencies)
• Hook C (priority=5, depends on A)

The topological sort would produce [A, C] correctly, but the final re-sort would produce [C, A], causing C to run before its dependency A.

After this fix, the result is [A, C] - dependencies always run before dependents, with priority acting as a tie-breaker for independent hooks.

Benefits

• Code consistency: Aligns with existing topological sort infrastructure
• Maintainability: Uses well-known Kahn's algorithm pattern
• Natural priority handling: PriorityQueue eliminates need for manual sorting

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[twisti-dev]

@copilot There is already a topological sort implemented under dev/slne/surf/surfapi/core/api/algorithms/kahn-topological-sort.kt maybe you can use that

[Copilot]

@copilot There is already a topological sort implemented under dev/slne/surf/surfapi/core/api/algorithms/kahn-topological-sort.kt maybe you can use that

Refactored to use Kahn's algorithm with a PriorityQueue for priority-based tie-breaking in commit 0d30fd4. The implementation follows the same pattern as the existing topological sort in kahn-topological-sort.kt, but uses a PriorityQueue instead of ArrayDeque to maintain priority ordering when multiple hooks have no dependencies.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[Copilot]

Pull request overview

This PR refactors the hook topological sort to fix a critical bug where dependencies were violated after sorting. The old DFS implementation correctly computed a topological order but then re-sorted by priority, breaking the dependency constraints.

Changes:

• Replaced DFS-based topological sort with Kahn's algorithm using a priority queue for tie-breaking
• Added cycle detection using graph traversal to identify circular dependencies
• Updated to use fastutil data structures consistent with existing codebase patterns

---

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

[Copilot]

The condition if (depClassName in hooksByClassName) checks against all hooks, but should check against validHooks only. If a dependency exists in hooksByClassName but was filtered out of validHooks (due to missing dependencies), it will create a graph node that doesn't correspond to a valid hook. This causes the algorithm to fail because the graph will have nodes that don't represent actual processable hooks. The check should ensure the dependency is in the validHooks set.

[Copilot]

The cycle detection logic is incorrect. The graph represents edges from dependencies to dependents (A -> B means B depends on A). To trace a cycle, we need to follow backwards through the dependency chain (from dependent to dependency), but this code follows successors (dependents) instead. This means the function won't correctly identify the actual cycle path. The algorithm needs to either build a reverse graph or use a different approach (like DFS) to correctly trace dependencies and find the cycle.