Skip to content

[Grove] Fix peer cyclic module dependency detection - #76

Open
MonzerMansour wants to merge 1 commit into
SchmiedmayerLab:mainfrom
MonzerMansour:fix/cyclic-module-dependencies
Open

[Grove] Fix peer cyclic module dependency detection#76
MonzerMansour wants to merge 1 commit into
SchmiedmayerLab:mainfrom
MonzerMansour:fix/cyclic-module-dependencies

Conversation

@MonzerMansour

@MonzerMansour MonzerMansour commented Aug 29, 2026

Copy link
Copy Markdown

Summary

  • Detect required peer dependency cycles (both modules in Configuration) that the search-stack algorithm missed.
  • Add a buildTypeOrder DFS safety net so optional/mixed peer cycles error with searchStackCycle instead of infinite recursion.
  • Document that mutual peer required dependencies also fail at resolve time.
  • Expand DependencyTests for peer, longer, optional, mixed, acyclic, and diamond graphs.

Motivation

Issue #18’s reproduction (ModuleA ↔ ModuleB both configured) crashed in recursive type-order DFS because require early-returns for already-queued peers and never extends the search stack.

Changes

Area Change
DependencyManager.swift Required-edge cycle detection + DFS visited/in-progress safety net
Module-Dependency.md Clarify peer mutual deps error at resolve
DependencyTests.swift Regression coverage for #18 cases

Test plan

  • xcodebuild test -scheme Grove-Tests -testPlan Grove -destination 'platform=macOS'TEST SUCCEEDED (62 tests)
  • peerModuleCycle, longerPeerModuleCycle, optional/mixed peer cycle tests green
  • Existing moduleCircle1 + acyclic/diamond cases green
  • CI green on this PR

Out of scope

  • Officially supporting cyclic dependencies (including treating optional cycles as valid)
  • Unrelated monorepo packages

Closes #18

Required mutual @dependency between modules both listed in Configuration
skipped search-stack cycle checks and infinite-recursed in type-order DFS.
Record required edges during resolve, detect peer cycles before ordering,
and add a DFS visited/in-progress safety net so optional peer cycles also
fail with searchStackCycle instead of crashing. Cover peer, longer, optional,
mixed, acyclic, and diamond cases in DependencyTests.

Closes SchmiedmayerLab#18

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e210aea0-579a-45b3-aa55-c51f58e10705


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

[Spezi] improved handling of cyclic dependencies between modules

1 participant